search

cossacklabs / themis

Easy to use cryptographic framework for data protection: secure messaging with forward secrecy and secure data storage. Has unified APIs across 14 platforms.
https://www.cossacklabs.com/themis
Apache License 2.0
1.85k stars 143 forks source link

fix the bugs found by cppcheck #130

Closed bryonglodencissp closed 8 years ago

bryonglodencissp commented 8 years ago

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.

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

bryonglodencissp commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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:

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 commented 8 years ago

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

bryonglodencissp commented 8 years ago

@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.