[PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()

Timo Sirainen· May 20, 2007, 02:24 UTC · lore

---
 diff.c |   51 ++++++++++++++++++++++-----------------------------
 1 files changed, 22 insertions(+), 29 deletions(-)

Show changes to diff.c +22 −29

diff --git a/diff.c b/diff.c
index 33297aa..4d8f4bc 100644
--- a/diff.c
+++ b/diff.c
@@ -9,6 +9,7 @@
 #include "xdiff-interface.h"
 #include "color.h"
 #include "attr.h"
+#include "str.h"
 
 #ifdef NO_FAST_WORKING_DIRECTORY
 #define FAST_WORKING_DIRECTORY 0
@@ -1823,14 +1824,14 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
 static void run_diff(struct diff_filepair *p, struct diff_options *o)
 {
 	const char *pgm = external_diff();
-	char msg[PATH_MAX*2+300], *xfrm_msg;
+	stringbuf(msg, PATH_MAX*2+300);
+	char *xfrm_msg;
 	struct diff_filespec *one;
 	struct diff_filespec *two;
 	const char *name;
 	const char *other;
 	char *name_munged, *other_munged;
 	int complete_rewrite = 0;
-	int len;
 
 	if (DIFF_PAIR_UNMERGED(p)) {
 		/* unmerged */
@@ -1847,30 +1848,26 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 	diff_fill_sha1_info(one);
 	diff_fill_sha1_info(two);
 
-	len = 0;
 	switch (p->status) {
 	case DIFF_STATUS_COPIED:
-		len += snprintf(msg + len, sizeof(msg) - len,
-				"similarity index %d%%\n"
-				"copy from %s\n"
-				"copy to %s\n",
-				(int)(0.5 + p->score * 100.0/MAX_SCORE),
-				name_munged, other_munged);
+		str_printfa(msg, "similarity index %d%%\n"
+			    "copy from %s\n"
+			    "copy to %s\n",
+			    (int)(0.5 + p->score * 100.0/MAX_SCORE),
+			    name_munged, other_munged);
 		break;
 	case DIFF_STATUS_RENAMED:
-		len += snprintf(msg + len, sizeof(msg) - len,
-				"similarity index %d%%\n"
-				"rename from %s\n"
-				"rename to %s\n",
-				(int)(0.5 + p->score * 100.0/MAX_SCORE),
-				name_munged, other_munged);
+		str_printfa(msg, "similarity index %d%%\n"
+			    "rename from %s\n"
+			    "rename to %s\n",
+			    (int)(0.5 + p->score * 100.0/MAX_SCORE),
+			    name_munged, other_munged);
 		break;
 	case DIFF_STATUS_MODIFIED:
 		if (p->score) {
-			len += snprintf(msg + len, sizeof(msg) - len,
-					"dissimilarity index %d%%\n",
-					(int)(0.5 + p->score *
-					      100.0/MAX_SCORE));
+			str_printfa(msg, "dissimilarity index %d%%\n",
+				    (int)(0.5 + p->score *
+					  100.0/MAX_SCORE));
 			complete_rewrite = 1;
 			break;
 		}
@@ -1889,19 +1886,15 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 			    (!fill_mmfile(&mf, two) && file_is_binary(two)))
 				abbrev = 40;
 		}
-		len += snprintf(msg + len, sizeof(msg) - len,
-				"index %.*s..%.*s",
-				abbrev, sha1_to_hex(one->sha1),
-				abbrev, sha1_to_hex(two->sha1));
+		str_printfa(msg, "index %.*s..%.*s",
+			    abbrev, sha1_to_hex(one->sha1),
+			    abbrev, sha1_to_hex(two->sha1));
 		if (one->mode == two->mode)
-			len += snprintf(msg + len, sizeof(msg) - len,
-					" %06o", one->mode);
-		len += snprintf(msg + len, sizeof(msg) - len, "\n");
+			str_printfa(msg, " %06o", one->mode);
+		str_append(msg, "\n");
 	}
 
-	if (len)
-		msg[--len] = 0;
-	xfrm_msg = len ? msg : NULL;
+	xfrm_msg = str_len(msg) ? str_c(msg) : NULL;
 
 	if (!pgm &&
 	    DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&

-- 
1.5.1.4

Petr Baudis· May 22, 2007, 13:43 UTC · re: Timo Sirainen · lore

Re: [PATCH 2/3] Use stringbuf to fix buffer overflows due to broken use of snprintf()

On Sun, May 20, 2007 at 04:24:39AM CEST, Timo Sirainen wrote:

Show 6 quoted lines

> @@ -1823,14 +1824,14 @@ static void diff_fill_sha1_info(struct diff_filespec *one)
>  static void run_diff(struct diff_filepair *p, struct diff_options *o)
>  {
>  	const char *pgm = external_diff();
> -	char msg[PATH_MAX*2+300], *xfrm_msg;
> +	stringbuf(msg, PATH_MAX*2+300);

I don't find this style of declaring a variable too clear; I think it might be worthwhile to make this stand out more and uppercase the stringbuf() macro.

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Ever try. Ever fail. No matter. // Try again. Fail again. Fail better.
		-- Samuel Beckett

patch, 3 partsUse stringbuf to fix buffer overflows due to broken use of snprintf()