fix the bugs found by cppcheck

[bryonglodencissp]

This is a complete list of current bugs that you or I can patch. We're providing this as reference. Most, if not all, are in the test suite and third-party tools.

It's also worth noting that we've done extensive studies, and 60% of the time our static analysis tool works all the time. Furthermore, sound and complete static analysis is shown to be undecidable, ergo we're led to a few false positives because we've adopted some unsound techniques in our tool.

• [jni/external/openssl/crypto/bn/bn_asm.c:160]: (error) Wrong number of parameters for macro 'mul_add'.
• [./jni/external/openssl/crypto/bn/bn_asm.c:160]: (error) Wrong number of parameters for macro 'mul_add'.
• [jni/external/openssl/crypto/des/cbc3_enc.c:96]: (error) Uninitialized variable: niv1
• [jni/external/openssl/crypto/des/cbc3_enc.c:97]: (error) Uninitialized variable: niv2
• [jni/external/openssl/crypto/des/ofb64ede.c:86]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/ofb64ede.c:87]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/ofb64ede.c:103]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/ofb64enc.c:84]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/ofb64enc.c:85]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/ofb64enc.c:96]: (error) Uninitialized variable: d
• [jni/external/openssl/crypto/des/times/aix.cc] -> [jni/external/openssl/crypto/des/times/aix.cc]: (error) syntax error
• [jni/external/openssl/crypto/modes/ccm128.c] -> [jni/external/openssl/crypto/modes/ccm128.c]: (error) Invalid value: 1UI64
• [jni/external/openssl/ssl/s2_clnt.c:432]: (error) syntax error
• [jni/external/openssl/ssl/s3_clnt.c:2969]: (error) Uninitialized variable: psk
• [src/soter/openssl/.#soter_hash.c] -> [src/soter/openssl/.#soter_hash.c]: (error) syntax error
• [src/wrappers/themis/jsthemis/secure_cell_context_imprint.cpp:72]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_context_imprint.cpp:76]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_context_imprint.cpp:93]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_context_imprint.cpp:97]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_seal.cpp:76]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_seal.cpp:80]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_seal.cpp:101]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_seal.cpp:105]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:78]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:84]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:79]: (error) Mismatching allocation and deallocation: token
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:85]: (error) Mismatching allocation and deallocation: token
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:110]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp:114]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_message.cpp:74]: (error) Mismatching allocation and deallocation: encrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:78]: (error) Mismatching allocation and deallocation: encrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:93]: (error) Mismatching allocation and deallocation: decrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:97]: (error) Mismatching allocation and deallocation: decrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:112]: (error) Mismatching allocation and deallocation: encrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:116]: (error) Mismatching allocation and deallocation: encrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:131]: (error) Mismatching allocation and deallocation: decrypted_data
• [src/wrappers/themis/jsthemis/secure_message.cpp:135]: (error) Mismatching allocation and deallocation: decrypted_data
• [src/wrappers/themis/jsthemis/secure_session.cpp:96]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_session.cpp:100]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_session.cpp:115]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_session.cpp:119]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_session.cpp:137]: (error) Mismatching allocation and deallocation: data
• [src/wrappers/themis/jsthemis/secure_session.cpp:141]: (error) Mismatching allocation and deallocation: data
• [tests/soter/nist-sts/src/assess.c:299]: (error) Resource leak: fp
• [tests/soter/nist-sts/src/assess.c:310]: (error) Memory leak: A
• [tests/soter/nist-sts/src/assess.c:310]: (error) Resource leak: fp
• [tests/soter/nist-sts/src/assess.c:334]: (error) Memory leak: A
• [tests/soter/nist-sts/src/assess.c:336]: (error) Resource leak: fp
• [tests/soter/nist-sts/src/assess.c:186]: (error) Memory leak: fp
• [tests/soter/nist-sts/src/assess.c:186]: (error) Memory leak: results
• [tests/soter/nist-sts/src/assess.c:320]: (error) Memory leak: T
• [tests/soter/nist-sts/src/nonOverlappingTemplateMatchings.c:133]: (error) Memory pointed to by 'sequence' is freed twice.
• [tests/soter/nist-sts/src/utilities.c:246]: (error) Deallocating a deallocated pointer: fp
• [tests/themis/themis_seccure_message.c:105]: (error) Memory leak: unwrapped_message
• [tests/themis/themis_seccure_message.c:53]: (error) Uninitialized variable: res
• [tests/tools/splint/source/src/cgrammar.c:7609]: (error) Memory allocation size is negative.
• [tests/tools/splint/source/src/llgrammar.c:4923]: (error) Memory allocation size is negative.
• [tests/tools/splint/source/src/signature.c:1815]: (error) Memory allocation size is negative.
• [tests/tools/splint/source/test/abstptr.c:17]: (error) Null pointer dereference: ap
• [tests/tools/splint/source/test/abstptr.c:18]: (error) Null pointer dereference: ap
• [tests/tools/splint/source/test/abstptr.c:17]: (error) Null pointer dereference
• [tests/tools/splint/source/test/abstptr.c:18]: (error) Null pointer dereference
• [tests/tools/splint/source/test/alias.c:21]: (error) Uninitialized variable: z
• [tests/tools/splint/source/test/alias2.c:17]: (error) Uninitialized variable: lx
• [tests/tools/splint/source/test/argorder.c:8]: (error) Expression '"%d %d",z++,z' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder.c:10]: (error) Expression '(i++,j++),i=3' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder2.c:19]: (error) Expression 'i++,i' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:7]: (error) Expression 'i=i++' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:8]: (error) Expression 'a[i]=i++' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:9]: (error) Expression 'a[i++]=i' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:10]: (error) Expression 'i++*i' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:11]: (error) Expression 'i*i++' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/argorder3.c:12]: (error) Expression '--i*++i' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/args.c:17]: (error) va_list 'args' was opened but not closed by va_end().
• [tests/tools/splint/source/test/arraylit.c:13] -> [tests/tools/splint/source/test/arraylit.c:5]: (error) Modifying string literal "unmodifiable .." directly or indirectly is undefined behaviour.
• [tests/tools/splint/source/test/buffertest1.c:5]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:8]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:10]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:11]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:12]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:13]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:20]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/buffertest1.c:21]: (error) Uninitialized variable: g
• [tests/tools/splint/source/test/chararraylit.c:13] -> [tests/tools/splint/source/test/chararraylit.c:9]: (error) Modifying string literal "abc" directly or indirectly is undefined behaviour.
• [tests/tools/splint/source/test/constannot.c:20] -> [tests/tools/splint/source/test/constannot.c:6]: (error) Array 'buf[20]' accessed at index 20, which is out of bounds.
• [tests/tools/splint/source/test/constannot.c:21] -> [tests/tools/splint/source/test/constannot.c:11]: (error) Array 'buf[20]' accessed at index 20, which is out of bounds.
• [tests/tools/splint/source/test/decl.c:13]: (error) Array 'buf[10]' accessed at index 10, which is out of bounds.
• [tests/tools/splint/source/test/divzero.c:3]: (error) Division by zero.
• [tests/tools/splint/source/test/enum.c:15]: (error) Uninitialized variable: a
• [tests/tools/splint/source/test/fileio/file.c:25]: (error) Uninitialized variable: res
• [tests/tools/splint/source/test/fileio/filerw.c:8]: (error) Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush in between result in undefined behaviour.
• [tests/tools/splint/source/test/fileio/filerw.c:12]: (error) Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush in between result in undefined behaviour.
• [tests/tools/splint/source/test/for/for.c:13]: (error) Array 't[11]' accessed at index 11, which is out of bounds.
• [tests/tools/splint/source/test/funcpointer.c:19]: (error) Address of function parameter 'i' returned.
• [tests/tools/splint/source/test/iter.h:7]: (error) Invalid number of character '{' when no macros are defined.
• [tests/tools/splint/source/test/iter2.c:14]: (error) Invalid number of character '{' when no macros are defined.
• [tests/tools/splint/source/test/libs.c:33]: (error) Read and write operations without a call to a positioning function (fseek, fsetpos or rewind) or fflush in between result in undefined behaviour.
• [tests/tools/splint/source/test/libs.c:47]: (error) Null pointer dereference
• [tests/tools/splint/source/test/libs.c:48]: (error) Null pointer dereference
• [tests/tools/splint/source/test/libs.c:51]: (error) Null pointer dereference
• [tests/tools/splint/source/test/malloc.c:5]: (error) Array 'ip[22]' accessed at index 88, which is out of bounds.
• [tests/tools/splint/source/test/malloc.c:15]: (error) Array 'ip[22]' accessed at index 22, which is out of bounds.
• [tests/tools/splint/source/test/malloc.c:26]: (error) Array 'ip[87]' accessed at index 87, which is out of bounds.
• [tests/tools/splint/source/test/malloc.c:35]: (error) Array 'ip[43]' accessed at index 86, which is out of bounds.
• [tests/tools/splint/source/test/malloc.c:3]: (error) The allocated size 89 is not a multiple of the underlying type's size.
• [tests/tools/splint/source/test/malloc.c:33]: (error) The allocated size 174 is not a multiple of the underlying type's size.
• [tests/tools/splint/source/test/manual/only.c:13]: (error) Dereferencing 'x' after it is deallocated / released
• [tests/tools/splint/source/test/manual/only.c:14]: (error) Memory leak: m
• [tests/tools/splint/source/test/manual/order.c:11]: (error) Expression 'x++*x' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/manual/order.c:13]: (error) Expression 'y[i]=i++' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/manual/sample.c:11]: (error) Memory leak: x
• [tests/tools/splint/source/test/manual/setChar.c:5]: (error) Array 'buf[10]' accessed at index 10, which is out of bounds.
• [tests/tools/splint/source/test/manual/stack.c:10]: (error) Address of local auto-variable assigned to a function parameter.
• [tests/tools/splint/source/test/manual/stack.c:12]: (error) Address of an auto-variable returned.
• [tests/tools/splint/source/test/manual/sumsquares.c:9]: (error) Expression '(i++)*(i++)' depends on order of evaluation of side effects
• [tests/tools/splint/source/test/merge.c:33]: (error) Dereferencing 'y' after it is deallocated / released
• [tests/tools/splint/source/test/mergestate/taintednm.c:24]: (error) Pointer to local array variable returned.
• [tests/tools/splint/source/test/metastate/file2.c:13]: (error) Resource leak: fle1
• [tests/tools/splint/source/test/metastate/file3.c:13]: (error) Resource leak: fle1
• [tests/tools/splint/source/test/metastate/file5.c:16]: (error) Returning/dereferencing 'fle' after it is deallocated / released
• [tests/tools/splint/source/test/metastate/file6.c:20]: (error) Returning/dereferencing 'res' after it is deallocated / released
• [tests/tools/splint/source/test/metastate/nullbranch.c:32]: (error) Resource leak: f
• [tests/tools/splint/source/test/moreBufferTests/strncatNotReallyGood.c:4]: (error) Uninitialized variable: buffer
• [tests/tools/splint/source/test/moreBufferTests2/fixedArrayType.c:9]: (error) Array 'buffer[10]' accessed at index 39, which is out of bounds.
• [tests/tools/splint/source/test/moreBufferTests2/initBlock.c:8]: (error) Array 'buf[10]' accessed at index 10, which is out of bounds.
• [tests/tools/splint/source/test/null1.c:19]: (error) Memory leak: y
• [tests/tools/splint/source/test/null1.c:67]: (error) Memory leak: z3
• [tests/tools/splint/source/test/nullterminatedtest/buggy_support1.c:47]: (error) Common realloc mistake: 'word' nulled but not freed upon failure
• [tests/tools/splint/source/test/nullterminatedtest/test1.c:10]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/nullterminatedtest/test1.c:11]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/nullterminatedtest/test1.c:13]: (error) Uninitialized variable: d
• [tests/tools/splint/source/test/nullterminatedtest/test3.c:14]: (error) Uninitialized variable: y
• [tests/tools/splint/source/test/nullterminatedtest/test3.c:16]: (error) Uninitialized variable: y
• [tests/tools/splint/source/test/nullterminatedtest/test3.c:19]: (error) Uninitialized variable: y
• [tests/tools/splint/source/test/nullterminatedtest/test3.c:14]: (error) Uninitialized variable: x
• [tests/tools/splint/source/test/nullterminatedtest/test3.c:34]: (error) Uninitialized variable: d
• [tests/tools/splint/source/test/outparam.c:48]: (error) Memory leak: t4
• [tests/tools/splint/source/test/outparam.c:8]: (error) Uninitialized variable: u1
• [tests/tools/splint/source/test/outparam.c:9]: (error) Uninitialized variable: u1
• [tests/tools/splint/source/test/outparam.c:37]: (error) Uninitialized variable: b
• [tests/tools/splint/source/test/outparam.c:48]: (error) Uninitialized variable: b
• [tests/tools/splint/source/test/outparam.c:37]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/outparam.c:39]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/outparam.c:40]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/outparam.c:39]: (error) Uninitialized variable: d
• [tests/tools/splint/source/test/outparam.c:40]: (error) Uninitialized variable: d
• [tests/tools/splint/source/test/outparam.c:41]: (error) Uninitialized variable: t
• [tests/tools/splint/source/test/outparam.c:42]: (error) Uninitialized variable: t2
• [tests/tools/splint/source/test/outparam.c:42]: (error) Uninitialized struct member: t2.a
• [tests/tools/splint/source/test/outparam.c:44]: (error) Uninitialized variable: t3
• [tests/tools/splint/source/test/sharing1.c:59]: (error) Memory leak: y2
• [tests/tools/splint/source/test/sharing1.c:59]: (error) Memory leak: y3
• [tests/tools/splint/source/test/sharing5.c:32]: (error) Memory leak: localp
• [tests/tools/splint/source/test/shifts.c:17]: (error) Shifting by a negative value is undefined behaviour
• [tests/tools/splint/source/test/simplebufferConstraintTests/sizeof.c:17]: (error) Array 'x[3]' accessed at index 3, which is out of bounds.
• [tests/tools/splint/source/test/simplebufferConstraintTests/test3.c:9]: (error) Array 'g[100]' accessed at index 101, which is out of bounds.
• [tests/tools/splint/source/test/simplebufferConstraintTests/test3.c:10]: (error) Array 'g[100]' accessed at index 100, which is out of bounds.
• [tests/tools/splint/source/test/sizeof.c:6]: (error) Array 'x[3]' accessed at index 3, which is out of bounds.
• [tests/tools/splint/source/test/sizeoftest/sizeof.c:6]: (error) Array 'x[3]' accessed at index 3, which is out of bounds.
• [tests/tools/splint/source/test/special.c:31]: (error) printf format string requires 3 parameters but only 2 are given.
• [tests/tools/splint/source/test/special.c:17]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/special.c:29]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/special.c:30]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/special.c:31]: (error) Uninitialized variable: c
• [tests/tools/splint/source/test/special.c:27]: (error) Uninitialized variable: s
• [tests/tools/splint/source/test/stack.c:9]: (error) Address of local auto-variable assigned to a function parameter.
• [tests/tools/splint/source/test/stack.c:20]: (error) Address of an auto-variable returned.
• [tests/tools/splint/source/test/stack.c:31]: (error) Pointer to local array variable returned.
• [tests/tools/splint/source/test/stack.c:35]: (error) Pointer to local array variable returned.
• [tests/tools/splint/source/test/strings.c:12] -> [tests/tools/splint/source/test/strings.c:24]: (error) Modifying string literal "hullo" directly or indirectly is undefined behaviour.
• [tests/tools/splint/source/test/t1.c:5]: (error) Memory leak: z
• [tests/tools/splint/source/test/tainted/tainted.c:19]: (error) Memory leak: t
• [tests/tools/splint/source/test/tainted/taintedimplicit.c:12]: (error) Allocation with taintme, system doesn't release it.
• [tests/tools/splint/source/test/tainted/taintedimplicit.c:17]: (error) Allocation with taintme, system doesn't release it.
• [tests/tools/splint/source/test/tainted/taintedmerge.c:16]: (error) Pointer to local array variable returned.
• [tests/tools/splint/source/test/tainted/test.c:8]: (error) Memory leak: t
• [tests/tools/splint/source/test/test.c:3]: (error) Uninitialized variable: ip
• [tests/tools/splint/source/test/tests2.2/arbints.c:22]: (error) Uninitialized variable: l
• [tests/tools/splint/source/test/tests2.2a/toralf.c:15]: (error) Uninitialized variable: i1
• [tests/tools/splint/source/test/tests2.2a/toralf.c:15]: (error) Uninitialized variable: f
• [tests/tools/splint/source/test/tests2.4/array.c:17]: (error) Memory leak: p
• [tests/tools/splint/source/test/tests2.4/ulrich.c:16]: (error) Memory leak: newblock
• [tests/tools/splint/source/test/tests2.5/boolbad.c:12]: (error) Uninitialized variable: foo
• [tests/tools/splint/source/test/tests2.5/impabsmodule.c:10]: (error) Uninitialized variable: var
• [tests/tools/splint/source/test/tests2.5/impabsmodule.c:10]: (error) Uninitialized variable: var2
• [tests/tools/splint/source/test/ud.c:10]: (error) Uninitialized variable: z
• [tests/tools/splint/source/test/ud.c:50]: (error) Uninitialized variable: z5
• [tests/tools/splint/source/test/ud.c:39]: (error) Uninitialized variable: j
• [tests/tools/splint/source/test/ud.c:40]: (error) Uninitialized variable: j
• [tests/tools/splint/source/test/ud.c:49]: (error) Uninitialized variable: i
• [tests/tools/splint/source/test/ullint.c:30]: (error) Array '_src[6]' accessed at index 7, which is out of bounds.

Found by https://github.com/bryongloden/cppcheck

[bryonglodencissp]

For what its worth, normally, we'd volunteer to patch all of the bugs for you! However, based on our analysis of over 200 applications, we can honestly say we've never seen so many bugs in one repository! With reference to the fact that these bugs are outside your mainline, thus non-critical, you may consider enlisting the help of novice git volunteers. http://up-for-grabs.net/#/ and http://www.firsttimersonly.com are two great sites we recommend.

In addition, we're personally interested in adding 'cppcheck' to repository build process, possibly as part of a continuous integration project. If this is something you or someone you know is interested in, we would like to gain this experience. Thank you.

[ignatk]

Thank you for this.

Funny to note that half of the reported issues are on the codebase of splint, which is another static analysis tool we looked into. We pursued the idea of having at least some analysis tool on the codebase, so will definitely checkout cppcheck.

[gene-eu-zz]

Thank you for your input, this is very valuable!

Well, apart from 1 real problem (nasal demons in C++ code), which is something @mnaza should fix urgently, everything else is external dependencies.

What I find funny (and alarming) is that the code, which triggered errors from cppcheck are:

• well-known and trusted static code analyzer
• NIST test suite
• Google's OpenSSL port used in many Android apps

In addition, we're personally interested in adding 'cppcheck' to repository build process, possibly as part of a continuous integration project. If this is something you or someone you know is interested in, we would like to gain this experience.

This is very interesting proposition, thanks a lot.

We will check out cppcheck ourselves, and, if we'll find it's output relevant for our codebase's history (we run external source code audits on milestones, so there's a lot of data to compare against), we'll definitely try to do this and would appreciate any help.

[gene-eu-zz]

In the interim, @mnaza, take care of undefined behavior in jsthemis, please.

[bryonglodencissp]

@secumod, @gene-eu & co, ensure that you're using the most up-to-date version of the splint, NIST test suite, and Google OpenSSL. We regularly volunteer our time to help third-party developers find and fix bugs. Once our patches get integrated into a newer version it's up to the individual users to pull and link the newer version.

It is odd & funny that splint had some many bugs reported against it. Without looking at the source code it's tough to determine if they are false positives. This would be my bet. Remember at the beginning of this post I cautioned against false positives. In security bug bounty hunting with any static analysis tool, false positives are par for the course.