{"thread":{"id":"8231","subject":"[PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()","startedAt":"2007-05-20T02:24:39Z","lastAt":"2007-05-22T13:43:06Z","messageCount":2,"participants":["Timo Sirainen","Petr Baudis"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"42668","messageId":"1179627879.32181.1286.camel@hurina","threadId":"8231","inReplyTo":null,"subject":"[PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()","fromName":"Timo Sirainen","fromEmail":"tss@iki.fi","sentAt":"2007-05-20T02:24:39Z","receivedAt":"2007-05-20T02:24:39Z","isPatch":true,"sender":{"key":"tss@iki.fi","avatar":null},"body":"---\n diff.c |   51 ++++++++++++++++++++++-----------------------------\n 1 files changed, 22 insertions(+), 29 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 33297aa..4d8f4bc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -9,6 +9,7 @@\n #include \"xdiff-interface.h\"\n #include \"color.h\"\n #include \"attr.h\"\n+#include \"str.h\"\n \n #ifdef NO_FAST_WORKING_DIRECTORY\n #define FAST_WORKING_DIRECTORY 0\n@@ -1823,14 +1824,14 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n static void run_diff(struct diff_filepair *p, struct diff_options *o)\n {\n \tconst char *pgm = external_diff();\n-\tchar msg[PATH_MAX*2+300], *xfrm_msg;\n+\tstringbuf(msg, PATH_MAX*2+300);\n+\tchar *xfrm_msg;\n \tstruct diff_filespec *one;\n \tstruct diff_filespec *two;\n \tconst char *name;\n \tconst char *other;\n \tchar *name_munged, *other_munged;\n \tint complete_rewrite = 0;\n-\tint len;\n \n \tif (DIFF_PAIR_UNMERGED(p)) {\n \t\t/* unmerged */\n@@ -1847,30 +1848,26 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \tdiff_fill_sha1_info(one);\n \tdiff_fill_sha1_info(two);\n \n-\tlen = 0;\n \tswitch (p->status) {\n \tcase DIFF_STATUS_COPIED:\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"similarity index %d%%\\n\"\n-\t\t\t\t\"copy from %s\\n\"\n-\t\t\t\t\"copy to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\tstr_printfa(msg, \"similarity index %d%%\\n\"\n+\t\t\t    \"copy from %s\\n\"\n+\t\t\t    \"copy to %s\\n\",\n+\t\t\t    (int)(0.5 + p->score * 100.0/MAX_SCORE),\n+\t\t\t    name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"similarity index %d%%\\n\"\n-\t\t\t\t\"rename from %s\\n\"\n-\t\t\t\t\"rename to %s\\n\",\n-\t\t\t\t(int)(0.5 + p->score * 100.0/MAX_SCORE),\n-\t\t\t\tname_munged, other_munged);\n+\t\tstr_printfa(msg, \"similarity index %d%%\\n\"\n+\t\t\t    \"rename from %s\\n\"\n+\t\t\t    \"rename to %s\\n\",\n+\t\t\t    (int)(0.5 + p->score * 100.0/MAX_SCORE),\n+\t\t\t    name_munged, other_munged);\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n \t\tif (p->score) {\n-\t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\t\"dissimilarity index %d%%\\n\",\n-\t\t\t\t\t(int)(0.5 + p->score *\n-\t\t\t\t\t      100.0/MAX_SCORE));\n+\t\t\tstr_printfa(msg, \"dissimilarity index %d%%\\n\",\n+\t\t\t\t    (int)(0.5 + p->score *\n+\t\t\t\t\t  100.0/MAX_SCORE));\n \t\t\tcomplete_rewrite = 1;\n \t\t\tbreak;\n \t\t}\n@@ -1889,19 +1886,15 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)\n \t\t\t    (!fill_mmfile(&mf, two) && file_is_binary(two)))\n \t\t\t\tabbrev = 40;\n \t\t}\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\"index %.*s..%.*s\",\n-\t\t\t\tabbrev, sha1_to_hex(one->sha1),\n-\t\t\t\tabbrev, sha1_to_hex(two->sha1));\n+\t\tstr_printfa(msg, \"index %.*s..%.*s\",\n+\t\t\t    abbrev, sha1_to_hex(one->sha1),\n+\t\t\t    abbrev, sha1_to_hex(two->sha1));\n \t\tif (one->mode == two->mode)\n-\t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n-\t\t\t\t\t\" %06o\", one->mode);\n-\t\tlen += snprintf(msg + len, sizeof(msg) - len, \"\\n\");\n+\t\t\tstr_printfa(msg, \" %06o\", one->mode);\n+\t\tstr_append(msg, \"\\n\");\n \t}\n \n-\tif (len)\n-\t\tmsg[--len] = 0;\n-\txfrm_msg = len ? msg : NULL;\n+\txfrm_msg = str_len(msg) ? str_c(msg) : NULL;\n \n \tif (!pgm &&\n \t    DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&\n-- \n1.5.1.4\n\n\n"},{"id":"42983","messageId":"20070522134306.GL4489@pasky.or.cz","threadId":"8231","inReplyTo":"1179627879.32181.1286.camel@hurina","subject":"Re: [PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2007-05-22T13:43:06Z","receivedAt":"2007-05-22T13:43:06Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Sun, May 20, 2007 at 04:24:39AM CEST, Timo Sirainen wrote:\n> @@ -1823,14 +1824,14 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n>  static void run_diff(struct diff_filepair *p, struct diff_options *o)\n>  {\n>  \tconst char *pgm = external_diff();\n> -\tchar msg[PATH_MAX*2+300], *xfrm_msg;\n> +\tstringbuf(msg, PATH_MAX*2+300);\n\nI don't find this style of declaring a variable too clear; I think it\nmight be worthwhile to make this stand out more and uppercase the\nstringbuf() macro.\n\n\n-- \n\t\t\t\tPetr \"Pasky\" Baudis\nStuff: http://pasky.or.cz/\nEver try. Ever fail. No matter. // Try again. Fail again. Fail better.\n\t\t-- Samuel Beckett\n"}]}