On 31.8.2007, at 0.35, Reece Dunn wrote:
The problem is that the git code is full of these random cases. It's simply a huge job to even try to verify the correctness of it. Even if someone did that and fixed all the problems, tomorrow there would be new ones because noone bothers to even try to avoid them. So there really isn't any point in trying to make git secure until the coding style changes.You don't want a manual check to do these kinds of checks. Not only is it a huge job, you have the human factor: people make mistakes. This is (in part) what the review process is for, but understanding how to identify code that is safe from buffer overruns, integer overflows and the like is a complex task. Also, it may work on 32-bit which has been verified, but not on 64-bit. It would be far better to specify the rules on how to detect these issues into a static analysis tool and have that do the checking for you. Therefore, it is possible to detect when new problems have been added into the codebase. Does sparse support identifying these issues?
Yes, it is a complex task. But if there did exist such a static analyzer tool already, it would probably show that half of the strcpy () calls (and others) in git are currently unsafe. Wouldn't help all that much I think.
The code should be easy to verify to be secure, and with some kind of a safe string API it's a lot easier than trying to figure out corner cases where strcpy() calls break.Why is it easier? If you have a fixed-size buffer, why not use strncpy, which is what a safe string API is essentially doing anyway?
Well, strncpy() is a pretty good example actually. A lot of people use it wrong, because they don't realize that it doesn't necessarily NUL-terminate the strings. So it's another example of a bad API that can be easily used wrong. And besides that, it also fills the rest of the buffer with NULs, which is almost always pointless waste of CPU.
And why is safe string API easier to verify? Here's an example: // see how easily you can use strncpy() to cause a buffer overflow: char buf[1024]; strncpy(buf, input, 2048);// see how impossible it is to cause a buffer overflow with my static string API:
STR_STATIC(str, 1024); sstr_append(str, input);Of course the above example is a simple one, but often when using libc string handling functions for building strings the code gets complex and there are all kinds of "is the buffer full already? what about now? and now? and now?" and with all of those checks it's easy to make mistakes.
The point is that if the APIs are (nearly) impossible to use insecurely, it's very easy to verify that the code is safe. The code doesn't get safe by lots of checks everywhere, it gets safe by placing a minimal amount of checks to small area of the code. The correctness of a few checks is a lot easier to verify.
Attachment:
PGP.sig
Description: This is a digitally signed message part
