Fix several buffer overflows using base::string_printf()

laf

@@ -1 +1 @@
-Subproject commit 381923970189e1af76f03f03ebf6877d7610451a
+Subproject commit 42ad03a9a30ebe619674aaf07ef27cf442133903

src/app/console.cpp

@@ -8,11 +8,13 @@
 #include "config.h"
 #endif
 
-#include <stdarg.h>
+#include <cstdarg>
-#include <stdio.h>
+#include <cstdio>
+#include <vector>
 
 #include "base/bind.h"
 #include "base/memory.h"
+#include "base/string.h"
 #include "ui/ui.h"
 
 #include "app/app.h"
@@ -106,15 +108,13 @@ Console::~Console()
 
 void Console::printf(const char* format, ...)
 {
-  char buf[4096];               // TODO warning buffer overflow
+  std::va_list ap;
-  va_list ap;
-
   va_start(ap, format);
-  vsprintf(buf, format, ap);
+  std::string msg = base::string_vprintf(format, ap);
   va_end(ap);
 
   if (!m_withUI || !wid_console) {
-    fputs(buf, stdout);
+    fputs(msg.c_str(), stdout);
     fflush(stdout);
     return;
   }
@@ -125,7 +125,7 @@ void Console::printf(const char* format, ...)
     ui::Manager::getDefault()->invalidate();
   }
 
-  /* update the textbox */
+  // Update the textbox
   if (!console_locked) {
     console_locked = true;
 
@@ -142,7 +142,7 @@ void Console::printf(const char* format, ...)
   std::string final;
   if (!text.empty())
     final += text;
-  final += buf;
+  final += msg;
 
   wid_textbox->setText(final.c_str());
 }

src/app/ui/status_bar.cpp

@@ -35,6 +35,7 @@
 #include "app/ui_context.h"
 #include "app/util/range_utils.h"
 #include "base/bind.h"
+#include "base/string.h"
 #include "doc/document_event.h"
 #include "doc/image.h"
 #include "doc/layer.h"
@@ -611,17 +612,15 @@ void StatusBar::showBackupIcon(BackupIcon icon)
   m_indicators->showBackupIcon(icon);
 }
 
-bool StatusBar::setStatusText(int msecs, const char *format, ...)
+bool StatusBar::setStatusText(int msecs, const char* format, ...)
 {
   if ((base::current_tick() > m_timeout) || (msecs > 0)) {
-    char buf[256];              // TODO warning buffer overflow
+    std::va_list ap;
-    va_list ap;
-
     va_start(ap, format);
-    vsprintf(buf, format, ap);
+    std::string msg = base::string_vprintf(format, ap);
     va_end(ap);
 
-    IndicatorsGeneration(m_indicators).add(buf);
+    IndicatorsGeneration(m_indicators).add(msg.c_str());
     m_timeout = base::current_tick() + msecs;
     return true;
   }
@@ -629,21 +628,18 @@ bool StatusBar::setStatusText(int msecs, const char *format, ...)
     return false;
 }
 
-void StatusBar::showTip(int msecs, const char *format, ...)
+void StatusBar::showTip(int msecs, const char* format, ...)
 {
-  char buf[256];                // TODO warning buffer overflow
+  std::va_list ap;
-  va_list ap;
-  int x, y;
-
   va_start(ap, format);
-  vsprintf(buf, format, ap);
+  std::string msg = base::string_vprintf(format, ap);
   va_end(ap);
 
   if (m_tipwindow == NULL) {
-    m_tipwindow = new CustomizedTipWindow(buf);
+    m_tipwindow = new CustomizedTipWindow(msg);
   }
   else {
-    m_tipwindow->setText(buf);
+    m_tipwindow->setText(msg);
   }
 
   m_tipwindow->setInterval(msecs);
@@ -654,14 +650,14 @@ void StatusBar::showTip(int msecs, const char *format, ...)
   m_tipwindow->openWindow();
   m_tipwindow->remapWindow();
 
-  x = bounds().x2() - m_tipwindow->bounds().w;
+  int x = bounds().x2() - m_tipwindow->bounds().w;
-  y = bounds().y - m_tipwindow->bounds().h;
+  int y = bounds().y - m_tipwindow->bounds().h;
   m_tipwindow->positionWindow(x, y);
 
   m_tipwindow->startTimer();
 
   // Set the text in status-bar (with inmediate timeout)
-  IndicatorsGeneration(m_indicators).add(buf);
+  IndicatorsGeneration(m_indicators).add(msg.c_str());
   m_timeout = base::current_tick();
 }
 

src/app/ui/status_bar.h

@@ -58,8 +58,8 @@ namespace app {
 
     void clearText();
 
-    bool setStatusText(int msecs, const char *format, ...);
+    bool setStatusText(int msecs, const char* format, ...);
-    void showTip(int msecs, const char *format, ...);
+    void showTip(int msecs, const char* format, ...);
     void showColor(int msecs, const char* text, const Color& color);
     void showTool(int msecs, tools::Tool* tool);
     void showSnapToGridWarning(bool state);

src/ui/alert.cpp

@@ -35,6 +35,7 @@
 #include "ui/alert.h"
 
 #include "base/bind.h"
+#include "base/string.h"
 #include "ui/box.h"
 #include "ui/button.h"
 #include "ui/grid.h"
@@ -73,34 +74,30 @@ void Alert::setProgress(double progress)
 
 AlertPtr Alert::create(const char* format, ...)
 {
-  char buf[4096];               // TODO warning buffer overflow
-  va_list ap;
-
   // Process arguments
+  std::va_list ap;
   va_start(ap, format);
-  vsprintf(buf, format, ap);
+  std::string msg = base::string_vprintf(format, ap);
   va_end(ap);
 
   // Create the alert window
   AlertPtr window(new Alert());
-  window->processString(buf);
+  window->processString(msg);
   return window;
 }
 
 // static
 int Alert::show(const char* format, ...)
 {
-  char buf[4096];               // TODO warning buffer overflow
-  va_list ap;
-
   // Process arguments
+  std::va_list ap;
   va_start(ap, format);
-  vsprintf(buf, format, ap);
+  std::string msg = base::string_vprintf(format, ap);
   va_end(ap);
 
   // Create the alert window
   AlertPtr window(new Alert());
-  window->processString(buf);
+  window->processString(msg);
   return window->show();
 }
 
@@ -124,19 +121,18 @@ int Alert::show()
   return ret;
 }
 
-void Alert::processString(char* buf)
+void Alert::processString(std::string& buf)
 {
   bool title = true;
   bool label = false;
   bool separator = false;
   bool button = false;
   int align = 0;
-  char *beg;
+  int c, beg;
-  int c, chr;
 
   // Process buffer
   c = 0;
-  beg = buf;
+  beg = 0;
   for (; ; c++) {
     if ((!buf[c]) ||
         ((buf[c] == buf[c+1]) &&
@@ -146,14 +142,13 @@ void Alert::processString(char* buf)
           (buf[c] == '-') ||
           (buf[c] == '|')))) {
       if (title || label || separator || button) {
-        chr = buf[c];
+        std::string item = buf.substr(beg, c-beg);
-        buf[c] = 0;
 
         if (title) {
-          setText(beg);
+          setText(item);
         }
         else if (label) {
-          Label* label = new Label(beg);
+          Label* label = new Label(item);
           label->setAlign(align);
           m_labels.push_back(label);
         }
@@ -162,7 +157,7 @@ void Alert::processString(char* buf)
         }
         else if (button) {
           char buttonId[256];
-          Button* button_widget = new Button(beg);
+          Button* button_widget = new Button(item);
           button_widget->setMinSize(gfx::Size(60*guiscale(), 0));
           m_buttons.push_back(button_widget);
 
@@ -170,8 +165,6 @@ void Alert::processString(char* buf)
           button_widget->setId(buttonId);
           button_widget->Click.connect(base::Bind<void>(&Window::closeWindow, this, button_widget));
         }
-
-        buf[c] = chr;
       }
 
       // Done
@@ -180,7 +173,7 @@ void Alert::processString(char* buf)
       // Next widget
       else {
         title = label = separator = button = false;
-        beg = buf+c+2;
+        beg = c+2;
         align = 0;
 
         switch (buf[c]) {

src/ui/alert.h

@@ -11,6 +11,7 @@
 #include "base/shared_ptr.h"
 #include "ui/window.h"
 
+#include <string>
 #include <vector>
 
 namespace ui {
@@ -34,7 +35,7 @@ namespace ui {
     static int show(const char* format, ...);
 
   private:
-    void processString(char* buf);
+    void processString(std::string& buf);
 
     Slider* m_progress;
     Box* m_progressPlaceholder;