Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
From: Jeff King <hidden>
Date: 2016-06-15 22:58:27
On Tue, Aug 20, 2013 at 01:15:02AM +0200, Erik Faye-Lund wrote:
This one seems real, although it's quite theoretical. It should only happen in cases where the log-message contains "%1", the initial malloc passed and reallocing two more bytes failed. However, what's much more of a disaster: "pos" is used after the call to realloc might have moved the memory!
Yeah, agreed on both counts.
quoted hunk ↗ jump to hunk
I guess something like this should fix both issues. Sorry about the lack of indentation, it seems Gmail has regressed, and the old compose mode is somehow gone... (also sorry for triple-posting to some of you, Gmail seems particularly broken today)diff --git a/compat/win32/syslog.c b/compat/win32/syslog.c index d015e43..0641f4e 100644 --- a/compat/win32/syslog.c +++ b/compat/win32/syslog.c@@ -43,11 +43,14 @@ void syslog(int priority, const char *fmt, ...) va_end(ap); while ((pos = strstr(str, "%1")) != NULL) { - str = realloc(str, ++str_len + 1); - if (!str) { + char *tmp = realloc(str, ++str_len + 1); + if (!tmp) { warning("realloc failed: '%s'", strerror(errno)); + free(str); return; } + pos = tmp + (pos - str); + str = tmp; memmove(pos + 2, pos + 1, strlen(pos)); pos[1] = ' '; }
Yes, that looks like the right solution. You could also convert "pos" to an integer index rather than a pointer (but then you end up adding it it to the pointer in the memmove call, which is probably just as ugly). -Peff