From: Philip Oakley <philipoakley@iee.org>
Date: Mon, 19 Aug 2013 22:55:53 GMT
Subject: Re: CPPCheck found 24 high risk bugs in Git v.1.8.3.4
Message-ID: <166132E40AA54EE387BA9B4558823C55@PhilipOakley>
In-Reply-To: <33FCCE7A2CD5494B88F3C5CDBC341DFD@PhilipOakley>

----- Original Message ----- 
From: "Philip Oakley" <philipoakley@iee.org>
> From: "Koch, Rick (Subcontractor)" <Rick.Koch@tbe.com>
> Sent: Monday, August 19, 2013 6:09 PM
>>I'm directing to this e-mail, as it seems to be the approved forum
>>for posting Git bugs. We ran CPPCheck against Git v.1.8.3.4
>>and found 24 high risk bugs. Please see the attachment xlsx.
>
>>Is there a method to post to the Git community to allow the
>>community to review and debunk as faults positive or develop
>>patches to fix lists code files?
>
>>v/r
>
>>Roderick (Rick) Koch
>>Information Assurance
>>Rick.Koch@tbe.com
>
> What OS version / CPPCheck version was this checked on?
>
> In case other readers don't have a .xlsx reader here is Rick's list in 
> plain text (may be white space damaged).
>
> I expect some will be false positives, and some will just be being too 
> cautious.
>
> Philip
>
> description resourceFilePath fileName lineNumber
>      nullPointer(CppCheck) \git-master\builtin\add.c add.c 286
>      wrongPrintfScanfArgNum(CppCheck) \git-master\builtin\fetch.c
> fetch.c 588
>      nullPointer(CppCheck) \git-master\builtin\ls-files.c ls-files.c
> 144
>      nullPointer(CppCheck) \git-master\builtin\merge.c merge.c 1208
>      doubleFree(CppCheck) \git-master\builtin\notes.c notes.c 275
>      nullPointer(CppCheck) \git-master\builtin\reflog.c reflog.c 437
>      uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
>      uninitvar(CppCheck) \git-master\builtin\rev-list.c rev-list.c 342
>      uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
> 2803
>      uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
> 2802
>      uninitvar(CppCheck) \git-master\compat\regex\regcomp.c regcomp.c
> 2805
>      memleakOnRealloc(CppCheck) \git-master\compat\win32\syslog.c
> syslog.c 46

This looks like a possible, based on 
http://bytes.com/topic/c/answers/215084-can-realloc-potentially-cause-memory-leak 
(Mac's reply, with tweaks)

"Misuse of realloc CAN cause a memory leak, but only when allocation 
fails"
"if realloc fails, the memory previously pointed to by 'str = 
realloc(str, ++str_len + 1)' will still be claimed, but you will have 
lost your only pointer to it, because realloc returns NULL on failure. 
This is a memory leak."

We (those using the compat function) then only provide a warning, so it 
could repeat endlessly.

Eric (cc'd) may be able to clarify if this is a possibility.

>      uninitvar(CppCheck)
> \git-master\contrib\examples\builtin-fetch--tool.c 
> builtin-fetch--tool.c
> 419
>      uninitvar(CppCheck) \git-master\fast-import.c fast-import.c 2917
>      nullPointer(CppCheck) \git-master\line-log.c line-log.c 638
>      nullPointer(CppCheck) \git-master\mailmap.c mailmap.c 156
>      uninitvar(CppCheck) \git-master\merge-recursive.c
> merge-recursive.c 1887
>      uninitvar(CppCheck) \git-master\notes.c notes.c 805
>      uninitvar(CppCheck) \git-master\notes.c notes.c 805
>      deallocret(CppCheck) \git-master\pretty.c pretty.c 677
>      resourceLeak(CppCheck) \git-master\refs.c refs.c 3041
>      doubleFree(CppCheck) \git-master\sequencer.c sequencer.c 924
>      nullPointer(CppCheck) \git-master\sha1_file.c sha1_file.c 125
>      doubleFree(CppCheck) \git-master\shell.c shell.c 130
>
> --
Philip