It is virtually impossible to write software that is completely free of bugs.
Most bugs are benign causing mere annoyance, if they are even noticed. Others
can cause larger problems like data loss and lost productivity. The most severe
can lead to exploitation resulting in network compromise, denial of service and
theft of intellectual property or customer data.
Through my professional career I've learned important processes and practices to build security in to each
phase of the development process. I can apply these processes to your software
performing security audits of you code base, dynamic analysis and penetration
testing for your application.
Fun Exercise
Here's a simple program that reads each line from a text file and prints it to the screen.
What's wrong with this code?
#include <stdio.h>
int main(int argc, char* argv[]))
{
char copy[80];
char line[80];
static const char filename = "file.txt";
FILE* file = fopen(filename, "r");
if(file != NULL)
{
while(fgets(line, sizeof(line), file) != NULL)
{
strncpy(copy, line, strlen(line));
printf(copy);
}
fclose(file);
}
return 0;
}
Click here for the answer.
Error #1
First, line 16 introduces a format string vulnerability.
In this case external data (from the file) is passed to the printf function thorugh the format parameter without having first been sanitized. If an attacker
can manipulate the contents of the external file, they could insert specially crafted format specifiers that could be used
to compromize the application.
User controlled data should never be passed to any of the printf family of functions through the format parameter. The above line
of code should be rewritten as:
printf("%s", copy);
Error #2
Second, it looks like the programmer is taking great pains to avoid a buffer overflow vulnerability. First, the line is read
from the file using fgets, limiting the amount read to the size of the destination buffer. This is good! This line is then
copied into a different buffer. The programmer keeping the habbit of limiting how much is copied by using strncpy instead of
strcpy. This is good!
However, the programmer is using strncpy incorrectly. strncpy will copy from the source to the destiniation until the specified
number of characters are reached OR the NULL terminator is encountered. If the specified number of characters is reached, the
copy operation terminates and a NULL terminator is NOT appended to the destination. Using strlen(src) to limit strncpy will NEVER
NULL terminate the destination buffer.
In this instance, the copy buffer is left without a NULL terminator. When this is passed to printf, a read buffer overflow is
extremely likely. The code should be rewritten in such a way that the destination buffer is terminated properly. For instance:
int term = sizeof(dest) - 1;
term = min(term, strlen(line));
strncpy(copy, line, term);
copy[term] = '\0';
