From 5981ab1d7050143f97636ad0ce786f01cc8b6035 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 09:41:08 +0100 Subject: [PATCH 1/6] coverity: add nodefs for abort macros Add nodefs for macros that abort the current flow due to errors. This includes macros that trigger on integer overflows and for the version check macro. This aids Coverity as we point out that these paths will cause a fatal error. --- script/user_nodefs.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/script/user_nodefs.h b/script/user_nodefs.h index 110f76851..f890511f4 100644 --- a/script/user_nodefs.h +++ b/script/user_nodefs.h @@ -6,3 +6,20 @@ */ #nodef GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { __coverity_panic__(); } + +#nodef GITERR_CHECK_ALLOC_ADD(out, one, two) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two)) { __coverity_panic__(); } + +#nodef GITERR_CHECK_ALLOC_ADD3(out, one, two, three) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), three)) { __coverity_panic__(); } + +#nodef GITERR_CHECK_ALLOC_ADD4(out, one, two, three, four) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), three) || \ + GIT_ADD_SIZET_OVERFLOW(out, *(out), four)) { __coverity_panic__(); } + +#nodef GITERR_CHECK_ALLOC_MULTIPLY(out, nelem, elsize) \ + if (GIT_MULTIPLY_SIZET_OVERFLOW(out, nelem, elsize)) { __coverity_panic__(); } + +#nodef GITERR_CHECK_VERSION(S,V,N) if (giterr__check_version(S,V,N) < 0) { __coverity_panic__(); } From 40f6f225175375b41317b0a4e98e8865600d1682 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 10:58:52 +0100 Subject: [PATCH 2/6] coverity: hint that string length is at least 2 When checking if a string is prefixed by a drive letter (e.g. "C:") we verify this by inspecting the first and second character of the string. Coverity thinks this is a defect as we do not check the string's length first, but in fact we only check the second character if the first character is part of the alphabet, that is it cannot be '\0'. Fix this by overriding the macro and explicitly checking the string's length. --- script/user_nodefs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/script/user_nodefs.h b/script/user_nodefs.h index f890511f4..3d25d92ec 100644 --- a/script/user_nodefs.h +++ b/script/user_nodefs.h @@ -23,3 +23,5 @@ if (GIT_MULTIPLY_SIZET_OVERFLOW(out, nelem, elsize)) { __coverity_panic__(); } #nodef GITERR_CHECK_VERSION(S,V,N) if (giterr__check_version(S,V,N) < 0) { __coverity_panic__(); } + +#nodef LOOKS_LIKE_DRIVE_PREFIX(S) (strlen(S) >= 2 && git__isalpha((S)[0]) && (S)[1] == ':') From 038d7af08595eabaa3d23da4703f25f4517af365 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 11:30:48 +0100 Subject: [PATCH 3/6] signature: use GITERR_CHECK_ALLOC to check for OOM situation When checking for out of memory situations we usually use the GITERR_CHECK_ALLOC macro. Besides conforming to our current code base it adds the benefit of silencing errors in Coverity due to Coverity handling the macro's error path as abort. --- src/signature.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/signature.c b/src/signature.c index 109476efe..d07c93323 100644 --- a/src/signature.c +++ b/src/signature.c @@ -79,10 +79,9 @@ int git_signature_new(git_signature **sig_out, const char *name, const char *ema GITERR_CHECK_ALLOC(p); p->name = extract_trimmed(name, strlen(name)); + GITERR_CHECK_ALLOC(p->name); p->email = extract_trimmed(email, strlen(email)); - - if (p->name == NULL || p->email == NULL) - return -1; /* oom */ + GITERR_CHECK_ALLOC(p->email); if (p->name[0] == '\0' || p->email[0] == '\0') { git_signature_free(p); From 704554cdf07c2cdbefbf586c1ea4e8af35c720db Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 11:37:48 +0100 Subject: [PATCH 4/6] transports: smart: fix memory leak on OOM path --- src/transports/smart_protocol.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 1d46d4bc9..6363378ec 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -108,6 +108,7 @@ static int append_symref(const char **out, git_vector *symrefs, const char *ptr) if (giterr_last()->klass != GITERR_NOMEMORY) goto on_invalid; + git__free(mapping); return error; } @@ -120,6 +121,7 @@ static int append_symref(const char **out, git_vector *symrefs, const char *ptr) on_invalid: giterr_set(GITERR_NET, "remote sent invalid symref"); git_refspec__free(mapping); + git__free(mapping); return -1; } From b0f7512f407284c9eec91681a7ced5c93e026644 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 11:46:10 +0100 Subject: [PATCH 5/6] transports: smart_pkt: fix memory leaks --- src/transports/smart_pkt.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index a6ae55d48..870f08497 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -271,6 +271,7 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len) line += 3; /* skip "ok " */ if (!(ptr = strchr(line, '\n'))) { giterr_set(GITERR_NET, "Invalid packet line"); + git__free(pkt); return -1; } len = ptr - line; @@ -314,6 +315,8 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) line = ptr + 1; if (!(ptr = strchr(line, '\n'))) { giterr_set(GITERR_NET, "Invalid packet line"); + git__free(pkt->ref); + git__free(pkt); return -1; } len = ptr - line; From 8a62bf11806d9118301999ff9c58d55057d06917 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 15 Feb 2016 11:28:33 +0100 Subject: [PATCH 6/6] netops: fix memory leak when an error occurs --- src/netops.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/netops.c b/src/netops.c index 5e8075597..c4241989f 100644 --- a/src/netops.c +++ b/src/netops.c @@ -261,6 +261,10 @@ int gitno_extract_url_parts( *path = git__substrdup(_path, u.field_data[UF_PATH].len); GITERR_CHECK_ALLOC(*path); } else { + git__free(*port); + *port = NULL; + git__free(*host); + *host = NULL; giterr_set(GITERR_NET, "invalid url, missing path"); return GIT_EINVALIDSPEC; }