From aa012e906542dbc533dbc5bafe1c4e0a47bdc59e Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sun, 16 Feb 2014 16:06:02 +0000 Subject: [PATCH 1/5] notes-utils: handle boolean notes.rewritemode correctly If we carry on after outputting config_error_nonbool then we're guaranteed to dereference a null pointer. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- notes-utils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/notes-utils.c b/notes-utils.c index 2975dcd581..4aa7023903 100644 --- a/notes-utils.c +++ b/notes-utils.c @@ -75,7 +75,7 @@ static int notes_rewrite_config(const char *k, const char *v, void *cb) return 0; } else if (!c->mode_from_env && !strcmp(k, "notes.rewritemode")) { if (!v) - config_error_nonbool(k); + return config_error_nonbool(k); c->combine = parse_combine_notes_fn(v); if (!c->combine) { error(_("Bad notes.rewriteMode value: '%s'"), v); From df5213b70d29e65aaff17d2577e42787e5a272bb Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sun, 16 Feb 2014 16:06:03 +0000 Subject: [PATCH 2/5] utf8: fix iconv error detection iconv(3) returns "(size_t) -1" on error. Make sure that we cast the "-1" properly when checking for this. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- utf8.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utf8.c b/utf8.c index 0d20e0acb2..24c3c5cedd 100644 --- a/utf8.c +++ b/utf8.c @@ -529,7 +529,7 @@ char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, int *outs while (1) { size_t cnt = iconv(conv, &cp, &insz, &outpos, &outsz); - if (cnt == -1) { + if (cnt == (size_t) -1) { size_t sofar; if (errno != E2BIG) { free(out); From a68a67dea399f15305b059aa36c007cfdde2890e Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sun, 16 Feb 2014 16:06:04 +0000 Subject: [PATCH 3/5] utf8: use correct type for values in interval table We treat these as unsigned everywhere and compare against unsigned values, so declare them using the typedef we already have for this. While we're here, fix the indentation as well. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- utf8.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utf8.c b/utf8.c index 24c3c5cedd..a831d50899 100644 --- a/utf8.c +++ b/utf8.c @@ -5,8 +5,8 @@ /* This code is originally from http://www.cl.cam.ac.uk/~mgk25/ucs/ */ struct interval { - int first; - int last; + ucs_char_t first; + ucs_char_t last; }; size_t display_mode_esc_sequence_len(const char *s) From d954828d45efbd4b53576e86066657e87391318d Mon Sep 17 00:00:00 2001 From: John Keeping Date: Sun, 16 Feb 2014 16:06:05 +0000 Subject: [PATCH 4/5] builtin/mv: don't use memory after free If 'src' already ends with a slash, then add_slash() will just return it, meaning that 'free(src_with_slash)' is actually 'free(src)'. Since we use 'src' later, this will result in use-after-free. In fact, this cannot happen because 'src' comes from internal_copy_pathspec() without the KEEP_TRAILING_SLASH flag, so any trailing '/' will have been stripped; but static analysis tools are not clever enough to realise this and so warn that 'src' could be used after having been free'd. Fix this by checking that 'src_w_slash' is indeed newly allocated memory. Signed-off-by: John Keeping Signed-off-by: Junio C Hamano --- builtin/mv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin/mv.c b/builtin/mv.c index 21c46d1636..7e26eb5229 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -162,7 +162,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) if (strncmp(path, src_w_slash, len_w_slash)) break; } - free((char *)src_w_slash); + if (src_w_slash != src) + free((char *)src_w_slash); if (last - first < 1) bad = _("source directory is empty"); From 78368f2c1ad342719ccf1e719bd5126ca6c7b68b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 18 Feb 2014 16:00:53 -0800 Subject: [PATCH 5/5] open_istream(): do not dereference NULL in the error case When stream-filter cannot be attached, it is expected to return NULL, and we should close the stream we opened and signal an error by returning NULL ourselves from this function. However, we attempted to dereference that NULL pointer between the point we detected the error and returned from the function. Brought-to-attention-by: John Keeping Signed-off-by: Junio C Hamano --- streaming.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/streaming.c b/streaming.c index d7c9f32f0c..2ff036a0fa 100644 --- a/streaming.c +++ b/streaming.c @@ -152,8 +152,10 @@ struct git_istream *open_istream(const unsigned char *sha1, if (filter) { /* Add "&& !is_null_stream_filter(filter)" for performance */ struct git_istream *nst = attach_stream_filter(st, filter); - if (!nst) + if (!nst) { close_istream(st); + return NULL; + } st = nst; }