diff options
author | Dirk Hohndel <dirk@hohndel.org> | 2013-03-03 17:53:43 -0800 |
---|---|---|
committer | Dirk Hohndel <dirk@hohndel.org> | 2013-03-03 20:18:23 -0800 |
commit | 01291929582ac573d7efd7fba3dbe061c9d70f9a (patch) | |
tree | cc836d5c2e40d26dd8f39dd5bad6c5193f2ef04d | |
parent | 93eeb03d67baac26da4153a163bf35567b106524 (diff) | |
download | subsurface-01291929582ac573d7efd7fba3dbe061c9d70f9a.tar.gz |
Try to capture some more potential buffer overflows caused by localization
A couple of these could clearly cause a crash just like the one fixed by
commit 00865f5a1e1a ("equipment.c: Fix potential buffer overflow in
size_data_funct()").
One would append user input to fixed length buffer without checking.
We were hardcoding the (correct) max path length in macos.c - replaced by
the actual OS constant.
But the vast majority are just extremely generous guesses how long
localized strings could possibly be.
Yes, this commit is likely leaning towards overkill. But we have now been
bitten by buffer overflow crashes twice that were caused by localization,
so I tried to go through all of the code and identify every possible
buffer that could be affected by this.
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
-rw-r--r-- | dive.c | 2 | ||||
-rw-r--r-- | divelist.c | 2 | ||||
-rw-r--r-- | equipment.c | 2 | ||||
-rw-r--r-- | gtk-gui.c | 4 | ||||
-rw-r--r-- | info.c | 4 | ||||
-rw-r--r-- | libdivecomputer.c | 4 | ||||
-rw-r--r-- | macos.c | 11 | ||||
-rw-r--r-- | parse-xml.c | 2 | ||||
-rw-r--r-- | print.c | 8 | ||||
-rw-r--r-- | profile.c | 4 | ||||
-rw-r--r-- | statistics.c | 6 | ||||
-rw-r--r-- | uemis-downloader.c | 4 | ||||
-rw-r--r-- | webservice.c | 8 |
13 files changed, 31 insertions, 30 deletions
@@ -354,7 +354,7 @@ static void match_standard_cylinder(cylinder_type_t *type) double cuft; int psi, len; const char *fmt; - char buffer[20], *p; + char buffer[40], *p; /* Do we already have a cylinder description? */ if (type->description) diff --git a/divelist.c b/divelist.c index 4fa817d0a..1912f0f1d 100644 --- a/divelist.c +++ b/divelist.c @@ -361,7 +361,7 @@ static void duration_data_func(GtkTreeViewColumn *col, { unsigned int sec; int idx; - char buffer[16]; + char buffer[40]; gtk_tree_model_get(model, iter, DIVE_INDEX, &idx, DIVE_DURATION, &sec, -1); if (idx < 0) diff --git a/equipment.c b/equipment.c index 19add3f4c..e7f11251f 100644 --- a/equipment.c +++ b/equipment.c @@ -1356,7 +1356,7 @@ static void weight_data_func(GtkTreeViewColumn *col, int idx = (long)data; int grams, decimals; double value; - char buffer[10]; + char buffer[64]; gtk_tree_model_get(model, iter, idx, &grams, -1); decimals = convert_weight(grams, &value); @@ -1793,7 +1793,7 @@ static gboolean profile_tooltip (GtkWidget *widget, gint x, gint y, gint tx = x - drawing_area->x; /* get transformed coordinates */ gint ty = y - drawing_area->y; gint width, height, time = -1; - char buffer[256], plot[256]; + char buffer[2048], plot[1024]; const char *event = ""; if (tx < 0 || ty < 0) @@ -2082,7 +2082,7 @@ void set_dc_nickname(struct dive *dive) { GtkWidget *dialog, *vbox, *entry, *frame, *label; char nickname[160] = ""; - char dialogtext[1024]; + char dialogtext[2048]; const char *name = nickname; struct divecomputer *dc = &dive->dc; @@ -793,7 +793,7 @@ static void dive_info_widget(GtkWidget *obox, struct dive *dive, struct dive_inf GtkWidget *image; #endif char buffer[256]; - char airtemp[6]; + char airtemp[10]; const char *unit; double value; @@ -1152,7 +1152,7 @@ int edit_dive_info(struct dive *dive, gboolean newdive) static GtkWidget *frame_box(GtkWidget *vbox, const char *fmt, ...) { va_list ap; - char buffer[64]; + char buffer[128]; GtkWidget *frame, *hbox; va_start(ap, fmt); diff --git a/libdivecomputer.c b/libdivecomputer.c index f4f82fb8b..9ad5df594 100644 --- a/libdivecomputer.c +++ b/libdivecomputer.c @@ -270,7 +270,7 @@ sample_cb(dc_sample_type_t type, dc_sample_value_t value, void *userdata) static void dev_info(device_data_t *devdata, const char *fmt, ...) { - static char buffer[256]; + static char buffer[1024]; va_list ap; va_start(ap, fmt); @@ -358,7 +358,7 @@ static inline int year(int year) static char *str_printf(const char *fmt, ...) { va_list args; - char buf[80]; + char buf[1024]; va_start(args, fmt); vsnprintf(buf, sizeof(buf)-1, fmt, args); @@ -7,6 +7,7 @@ #include <CoreServices/CoreServices.h> #include <mach-o/dyld.h> #include "gtkosxapplication.h" +#include <sys/syslimits.h> static GtkosxApplication *osx_app; @@ -148,9 +149,9 @@ int subsurface_fill_device_list(GtkListStore *store) const char *subsurface_icon_name() { - static char path[1024]; + static char path[PATH_MAX]; - snprintf(path, 1024, "%s/%s", gtkosx_application_get_resource_path(), ICON_NAME); + snprintf(path, sizeof(path), "%s/%s", gtkosx_application_get_resource_path(), ICON_NAME); return path; } @@ -173,7 +174,7 @@ const char *subsurface_gettext_domainpath(char *argv0) { /* on a Mac we ignore the argv0 argument and instead use the resource_path * to figure out where to find the translation files */ - static char buffer[256]; + static char buffer[PATH_MAX]; const char *resource_path = gtkosx_application_get_resource_path(); if (resource_path) { snprintf(buffer, sizeof(buffer), "%s/share/locale", resource_path); @@ -192,9 +193,9 @@ void subsurface_ui_setup(GtkSettings *settings, GtkWidget *menubar, GtkWidget *vbox, GtkUIManager *ui_manager) { GtkWidget *menu_item, *sep; - static char path[1024]; + static char path[PATH_MAX]; - snprintf(path, 1024, "%s/xslt", gtkosx_application_get_resource_path()); + snprintf(path, sizeof(path), "%s/xslt", gtkosx_application_get_resource_path()); setenv("SUBSURFACE_XSLT_PATH", path, TRUE); g_object_set(G_OBJECT(settings), "gtk-font-name", UI_FONT, NULL); diff --git a/parse-xml.c b/parse-xml.c index b1bb2efb9..23f0ddaf4 100644 --- a/parse-xml.c +++ b/parse-xml.c @@ -806,7 +806,7 @@ static const char *country, *city; static void divinglog_place(char *place, void *_location) { char **location = _location; - char buffer[256], *p; + char buffer[1024], *p; int len; len = snprintf(buffer, sizeof(buffer), @@ -69,7 +69,7 @@ static void show_dive_header(struct dive *dive, cairo_t *cr, double w, PangoLayout *layout; PangoRectangle ink_ext, logic_ext; struct tm tm; - char buffer[160], divenr[40], *people; + char buffer[512], divenr[80], *people; maxwidth = w * PANGO_SCALE; maxheight = h * PANGO_SCALE * 0.9; @@ -355,7 +355,7 @@ static void print_weight_data (struct dive *dive, cairo_t *cr, int maxwidth, int /* Print the dive OTUs */ static void print_otus (struct dive *dive, cairo_t *cr, PangoLayout *layout, int maxwidth) { - char buffer[40]; + char buffer[128]; cairo_move_to (cr,(maxwidth*0.05) / ((double) PANGO_SCALE), 0); snprintf(buffer, sizeof(buffer), _("OTU")); @@ -370,7 +370,7 @@ static void print_otus (struct dive *dive, cairo_t *cr, PangoLayout *layout, int /* Print the dive maxCNS */ static void print_cns (struct dive *dive, cairo_t *cr, PangoLayout *layout, int maxwidth) { - char buffer[40]; + char buffer[128]; cairo_move_to (cr,(maxwidth*0.05) / ((double) PANGO_SCALE), 0); @@ -389,7 +389,7 @@ static void print_SAC (struct dive *dive, cairo_t *cr, PangoLayout *layout, int double sac; int decimals; const char *unit; - char buffer[40]; + char buffer[128]; cairo_move_to (cr,(maxwidth*0.05) / ((double) PANGO_SCALE), 0); snprintf(buffer, sizeof(buffer), _("SAC")); @@ -268,7 +268,7 @@ static void plot_text(struct graphics_context *gc, const text_render_options_t * cairo_font_extents_t fe; cairo_text_extents_t extents; double dx, dy; - char buffer[80]; + char buffer[256]; va_list args; va_start(args, fmt); @@ -346,7 +346,7 @@ static void plot_one_event(struct graphics_context *gc, struct plot_info *pi, st { int i, depth = 0; int x,y; - char buffer[80]; + char buffer[256]; /* is plotting this event disabled? */ if (event->name) { diff --git a/statistics.c b/statistics.c index b6b618fbd..0ce8f6afe 100644 --- a/statistics.c +++ b/statistics.c @@ -192,7 +192,7 @@ static void init_tree() /* Add all the columns to the tree view */ for (i = 0; i < N_COLUMNS; ++i) { - char buf[80]; + char buf[256]; column = gtk_tree_view_column_new(); snprintf(buf, sizeof(buf), "%s\n%s", _(columnstop[i]), columnsbot[i]); gtk_tree_view_column_set_title(column, buf); @@ -500,7 +500,7 @@ void process_selected_dives(void) static void set_label(GtkWidget *w, const char *fmt, ...) { - char buf[80]; + char buf[256]; va_list args; va_start(args, fmt); @@ -531,7 +531,7 @@ static char *get_time_string(int seconds, int maxdays) * to dive data, but for consistency we don't. */ static void show_single_dive_stats(struct dive *dive) { - char buf[80]; + char buf[256]; double value; int decimals; const char *unit; diff --git a/uemis-downloader.c b/uemis-downloader.c index 91eae3145..9b54b0f3c 100644 --- a/uemis-downloader.c +++ b/uemis-downloader.c @@ -145,7 +145,7 @@ static struct dive *uemis_start_dive(uint32_t deviceid) /* send text to the importer progress bar */ static void uemis_info(const char *fmt, ...) { - static char buffer[40]; + static char buffer[256]; va_list ap; va_start(ap, fmt); @@ -544,7 +544,7 @@ static void parse_divespot(char *buf) char *bp = buf + 1; char *tp = next_token(&bp); char *tag, *type, *val; - char locationstring[255] = ""; + char locationstring[1024] = ""; int divespot, len; double latitude, longitude; diff --git a/webservice.c b/webservice.c index be3e1d273..9cdd75ced 100644 --- a/webservice.c +++ b/webservice.c @@ -49,11 +49,11 @@ gboolean webservice_request_user_xml(const gchar *user_id, SoupMessage *msg; SoupSession *session; gboolean ret = FALSE; - gchar url[80] = {0}; + gchar url[256] = {0}; session = soup_session_async_new(); strcat(url, "http://api.hohndel.org/api/dive/get/?login="); - strcat(url, user_id); + strncat(url, user_id, sizeof(url) - strlen(url) - 1); msg = soup_message_new("GET", url); soup_message_headers_append(msg->request_headers, "Accept", "text/xml"); soup_session_send_message(session, msg); @@ -115,7 +115,7 @@ static void download_dialog_connect_cb(GtkWidget *w, gpointer data) guint len, status_connect, status_xml; gchar *xmldata; gboolean ret; - gchar err[128] = {0}; + gchar err[256] = {0}; gtk_label_set_text(GTK_LABEL(state->status), _("Connecting...")); gtk_widget_set_sensitive(state->apply, FALSE); @@ -126,7 +126,7 @@ static void download_dialog_connect_cb(GtkWidget *w, gpointer data) if (status_xml != DD_STATUS_OK) ret = FALSE; } else { - sprintf(err, "%s %u!", download_dialog_status_text(DD_STATUS_ERROR_CONNECT), status_connect); + snprintf(err, sizeof(err), "%s %u!", download_dialog_status_text(DD_STATUS_ERROR_CONNECT), status_connect); gtk_label_set_text(GTK_LABEL(state->status), err); } state->xmldata = xmldata; |