Review Code for Security Defects

In a limited time constraint, what should we look for when reviewing code for security bugs? In this MSDN article, the author suggested to check for the following bugs in code review.

Buffer Overruns in C/C++

void function(char *p) {
char buff[16];
...
strcpy(buff,p);
...
}

When see code like this, trace the variable p back to its source. If it came from an untrusted input, or is not checked for validity close to the point at which it's copied, then it's a security bug.

while (*s != '\\')
*d++ = *s++;

The loop will stop when pointer s hits '\', but pointer d is not checked. The buffer to which pointer d points may be overrun.

Integer Overflows in C/C++

void function(char *b1, size_t c1, char *b2, size_t c2) {
const size_t MAX = 48;
if (c1+c2 > MAX) return;
char *pBuff = new char[MAX];
memcpy(pBuff, b1, c1);
memcpy(pBuff+c1, b2, c2);
}

We may have an integer overflow bug, when the sum of c1 and c2 could exceed the maximum of an integer, i.e., 2^32-1. A rule of thumb is suggested: if you perform a mathematical operation in an expression that is used in a comparison, then you have a potential to overflow and underflow data.

Data Access Code
Don't hard code passwords in connection strings or connect to database using administrative accounts. Also, A database query using string concatenation is a potential security defect and should be fixed by using parameterized queries.

Web Page Code

Hello, <% Response.Write(Request.QueryString("Name")) %>

For code like this, be aware of cross-site scripting issues.

Secrets and Cryptography
Don't store secret data in code, such as passwords and cryptographic keys. Don't create one's own magic cryptographic algorithms, like using an embedded key to XOR a data stream.