68

A person has good knowledge of overall security risks, knows what OWASP Top 10 vulnerabilities are, and has certifications like CEH, CISSP, OSCP, etc. which are more black-box testing. And also he has gone through the OWASP Testing Guide, Code Review Guide, etc. and cheat sheets. Will he be able to perform secure code review without knowledge of multiple programming languages and mastery over them?

  • 57
    He will not be able to perform a thorough secure code review if he does not know the languages in which the code to be reviewed has been written. – Xander Nov 24 '15 at 18:28
  • 20
    Please don't write in the third person :P – veryRandomMe Nov 24 '15 at 20:38
  • 30
    Can you catch and understand all the exploits at underhanded-c.org (which are fairly short and explained in detail)? That's one language. – user15392 Nov 24 '15 at 20:53
  • 6
    @drewbenn Another great example is escape.alf.nu - it's incredibly hard to prevent XSS using sanitization. Most of the tasks require you to know exactly how JavaScript and HTML work and interact. – Luaan Nov 25 '15 at 9:04
  • 3
    "To understand somebody's else code you need to be twice as good as when writing it from scratch." – Agent_L Nov 26 '15 at 16:10
116

It depends on what is meant by "secure source code analysis." One can do anything one pleases. The issue, I presume, is when someone else has asked for something called "secure source code analysis," and one wonders why one is not qualified for it.

In many cases, such analysis must be done by a Subject Matter Expert (SME). In the final product, a SME will deliver a statement basically saying "I declare this code to be secure," with an understanding that is a more profound statement than "I looked for a bunch of known patterns, and found no problems."

If you were interested in the authentic translation of a Chinese philosophy, would you trust an individual who knew a great deal about philosophy, and had a bunch of cheat sheets to help decipher it, but did not actually know Chinese?

One great example that comes to mind is a bug that hit a SQL engine. Forgive me for not having the name of which engine, or which version so you can verify, I have had trouble finding it since. However, the error was poignant. The error was in code that looked like this:

int storeDataInCircularBuffer(Buffer* dest, const char* src, size_t length)
{
    if (dest->putPtr + length < dest->putPtr)
        return ERROR; // prevent buffer overflow caused by overflow
    if (dest->putPtr + length > dest->endPtr) {
        ... // write the data in two parts
        return OK;
    } else {
        ... // write the data in one part
        return OK;
    }
}

This code was intended to be part of a circular buffer. In a circular buffer when you reach the end of the buffer, you wrap around. Sometimes this forces you to break the incoming message into two parts, which is okay. However, in this SQL program, they were concerned with the case where length could be large enough to cause dest->putPtr + length to overflow, creating an opportunity for a buffer overflow because the next check wouldn't work right. So they put in a test: if (dest->putPtr + length < dest->putPtr). Their logic was that the only way this statement could ever be true is if an overflow occurred, thus we catch the overflow.

This created a security hole that actually got exploited, and had to be patched. Why? Well, unbeknownst to the original author, the C++ spec declares that overflow of a pointer is undefined behavior, meaning the compiler can do anything it wants. As it so happened, when the original author tested it, gcc actually emitted the correct code. However, a few versions later, gcc did have optimizations to leverage this. It saw that there was no defined behavior where that if statement could pass its test, and optimized it out!

Thus, for a few versions, people had SQL servers which had an exploit, even though the code had explicit checks to prevent said exploit!

Fundamentally programming languages are very powerful tools that can bite the developer with ease. Analyzing whether this will occur does require a solid foundation in the language in question.

(Edit: Greg Bacon was great enough to dig up a CERT warning on this: Vulnerability Note VU#162289 C compilers may silently discard some wraparound checks., and also this related one. Thanks Greg!)

  • Comments are not for extended discussion; this conversation has been moved to chat. – Rory Alsop Nov 26 '15 at 23:07
  • Yeah. I should note that without knowing the language, an analyst might not even know what the programmer is doing (let alone be able to find all the security problems), or why. Some languages have some very interesting features that are not obvious if you don't know the language well. Hopefully most stuff like that would have comments for the analyst, but I wouldn't count completely on comments to guide you. – Shule Nov 27 '15 at 9:20
  • 1
    This kind of behaviour from a compiler always makes me wonder: given that the compiler knows that there is undefined behaviour, and that 100% of the time undefined behaviour is something you don't want in your code, couldn't the compiler warn when doing such a thing? This could prevent tons of bugs.... – Bakuriu Nov 27 '15 at 11:57
  • 2
    @Bakuriu: In C++ such "can't happen" cases routinely come up, without any bug being present, when specializing templates or optimizing inlined function calls with constant parameters -- and in those cases optimizing them away can be absolutely crucial for performance. It would be quite hard for a compiler to distinguish reliably between "programmer wrote something undefined" and "programmer used a valid generic function in a valid way that I can generate better code for than the generic case" and report warnings only in the former case. – Henning Makholm Nov 27 '15 at 14:19
27

I think you need to be a good programmer to be successful, so I'd recommend becoming one. There may be lots of things that your toolkit / scanner misses. I honestly don't recommend relying completely on tools scan your code for you, as exploits change constantly, and someone may have coded in a way where the scanner can't detect the vulnerabilities.

The ability to step through code and see how it works, and how it shouldn't work, is fundamental to secure software development. Having a developer aware of security issues is exactly what you want when it comes to producing a solid product, and exactly what you need during a code review.

While yes, you can point and click and check for vulnerabilities with your scanners and toolkits, it's not going to be very effective in the grand scheme of things. Do you know how much more effective you'd be if you could look at code yourself and determine whether it's good or bad? Waaaay better.

Don't try to pass a secure code review if you don't know what you're doing, but don't outright give up on the idea if you feel you aren't at a point where you can do a good review. I recommend trying to learn by creating your own mockup secure code reviews, and going through it a few times to ensure everything is okay.

But still, you should definitely learn not only to code, but to code well.

  • 2
    Also note that scanners only scan for known vulnerabilities/ atack vectors – Pinoniq Nov 24 '15 at 23:35
14

It's doubtful that a security expert would be effective at performing source code analysis without also being a skilled programmer. Many vulnerabilities are the result of technical or syntactical coding practices that are misused in some minor way. A missing semi-colon, an equal sign instead of a double-equals, an array boundary that is doubly defined on day one but one version is changed in the subsequent release, missing brackets, memory leaks, all have led to vulnerabilities. An experienced developer may see these, but a novice likely would not.

What your security expert should be doing is encouraging the engineers to use automated scanning tools such as static code analyzers, fuzz testers, and dynamic app verifiers. Help the engineering teams to understand input validation, injection attacks, trust boundaries. Build awareness that security defects need to be prioritized appropriately and addressed quickly. Schedule and conduct pen tests. And most important, get the engineers to do code reviews of each others' work.

Yes, the security expert should be able to read the code, but that does not mean he is qualified to be the arbiter of code security.

12

It depends on your expectations. Security vulnerabilities caused by design problems (i.e. missing CSRF protection, only rudimentary implementation of a protocol etc) can probably be found if the tester has a deep knowledge of security concepts, even if (s)he is only able to follow the code flow without having a deeper knowledge of the specific programming language.

But language specific security problems like buffer overflows, off-by-one errors, handling of unicode or \0, problems caused by the size of the data types and signed vs. unsigned etc will not be found if the tester has no deeper knowledge of the language, of bad practices and of typical insecurity patterns specific to the language. Take the history of Java vulnerabilities as an example, where not even the Java expert developers noticed the holes they punched into the sandbox of the language by adding reflection to the language and only external experts with a deep understanding of the language internals detected the flaws.

3

Does one need to be a good programmer to perform secure source code analysis?

No.

Will he be able to perform secure code review without knowledge of multiple programming languages and mastery over them?

No.

There's more to programming than expertise in the details of how various languages work. It's one of the things you need to be a good programmer, and it's also one of the things you need to be able to analyse source code from a security perspective (or any other quality).

So while you don't need to be a good programmer, you do need mastery of the languages involved.

  • So as per you, Mastery of the language = ? – Krishna Pandey Nov 25 '15 at 12:01
  • @K.P. Sorry, I don't follow you. – Jon Hanna Nov 25 '15 at 12:05
  • My understanding is, mastery of the language should make someone good programmer. isn't so? – Krishna Pandey Nov 25 '15 at 12:08
  • 4
    Hardly. You could know every nuance of a language and be no good at design, problem-solving, algorithm choice, invariant definition, test development, or most of debugging. Mastery of the language is certainly important, but it's not the most vital thing. Indeed perhaps less so for programming than source analysis (a programmer who doesn't know of a particular feature can attack the problem another way, someone analysing a program who doesn't know of a particular feature that was actually used had better learn it to be able to analyse the implications). – Jon Hanna Nov 25 '15 at 12:19
  • 1
    Design knowledge is probably needed, too, though, since vulnerabilities can sometimes lie in a poor design rather than a faulty implementation. – reirab Nov 25 '15 at 16:01
3

Not only does secure code review require knowledge of the high level language, but also of the compiler options and HOW THE CODE ACTUALLY WILL WORK ON THE CPU! High level languages are efficient to write code in because they hide a lot of the complexity. But many errors and bugs hide within the complexity. As pointed out in another answer, compilers try to do the right thing, but you really need to understand what is going on by disassembling the code and developing deep understanding of how it works.

This understanding is also required with scripting languages like JavaScript that interpret the high level code to CPU instructions and memory allocation. Unfortunately, this review would be platform dependent. See for example at https://en.m.wikipedia.org/wiki/Interpreted_language.

  • 2
    This is the true answer. 15 years ago security experts found bugs, wrote exploits, wrote papers about new techniques, etc. Now they just get "certified", throw around a few terms that they might only understand on a conceptual level (eg. buffer overflow, have you ever written one?), and think they're the same as the pioneer hackers. Two entirely different ballgames. Passing a tool's check isn't the same as being secure. Neither is surviving a fuzzer. – horse hair Nov 25 '15 at 9:38
  • Not true if your focusing on many modern languages such as Java and JavaScript. – Neil Smithline Nov 25 '15 at 15:30
  • 1
    @NeilSmithline In those cases, it's even worse because how it will run on the CPU now depends on which CPU it's running on (and, in the case of JavaScript, which interpreter it's running in.) – reirab Nov 25 '15 at 15:57
  • 1
    @NeilSmithline - I believe you would still need an understanding of how the scripting languages like JavaScript interpret the high level code to CPU instructions and memory allocation to be able to say with certainty that a bit of code is secure. Unfortunately, this review would be platform dependent. – Stone True Nov 25 '15 at 16:00
  • Hmm... It doesn't seem that way to me but obviously others agree with you. – Neil Smithline Nov 25 '15 at 16:19
1

In order to properly figure out the danger of side-channel attacks, you need to know hardware. There are really ugly side-channel attacks, like running a non-privileged process on a multi-CPU setup in parallel with a privileged one doing some encryption/decryption task and probing which shared cache lines get dirtied in what kind of temporal pattern or by timing the respective delivery of specific pattern sequences getting encrypted.

With encryption algorithms being under intense scrutiny of mathematicians and other theoreticists, side channel attacks are increasingly important ways of poking the game open again. The disadvantage is that they have to be crafted towards a particular implementation, code, and hardware.

  • 5
    Doesn't really address programming skill, the main point of the question. – Neil Smithline Nov 25 '15 at 15:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.