Secure Programming

From ReactOS Wiki
Revision as of 15:26, 15 May 2014 by Zehnvor (talk | contribs)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to: navigation, search

Common mistakes in programming:

The majority of programming errors that lead to security vulnerabilities are due to assumptions made about the code or the data. Hopefully this will make people more aware.

Buffers

These are what result in buffer overflow attacks. If you define the length of a buffer to be a certain length and an attacker can somehow manage to insert more data then the code is vulnerable e.g.:

 char buff[10];
 strcpy(buff, user_supplied_data);

Calculating Buffer Lengths

  • Can an attack induce a spurious null in the buffer, resulting in strlen reporting an incorrect value?
  • Remember: strlen will return length of the string excluding the null terminator in size_t.
  • Remember: sizeof will return the length of the buffer as defined in bytes.

Don't assume lengths.

Definition of Length

Different functions define length in different ways (n is the number of bytes specified in the arguments) e.g.:

  • strncat will add up to n bytes and append a null.
  • snprintf will add up to n-1 bytes and append a null.
  • strncpy will null all bytes and then add up to n bytes.

Check the function specifications!

Off By One

A very common mistake where the programmer gets confused between the size of the buffer, the size available for data and counting from zero.

  • A buffer defined as length n has an index from 0 to n-1:
 char buffer[10];
 buffer[10] = 0x61;  /* outside the buffer! */
  • A buffer defined as length n has n-1 space for data:
 char buffer[10];
 strcpy(buffer, "EXPLOIT123");  /* now buffer is not null terminated */

A buffer defined with length MAX_PATH can store MAX_PATH-1 characters.

Control Characters

Can an attacker insert arbitrary control characters in manipulated data? Common example are '\n' '%s' '%i' and '\0'.

Directory Traversal

If you are concatenating strings make sure you are stripping out '../' '..\\':

 buffer1 = "C:\\SomeDirectory\\Somewhere\\";
 buffer2 = "..\\..\\ReactOS\\System32\\";
 strcat(buffer1, buffer2);
 /* Now buffer1 = "C:\\SomeDirectory\\Somewhere\\..\\..\\ReactOS\\System32\\" = "C:\\ReactOS\\System32\\" */

Format String

This is where the first parameter (the format) to a printf type function is user specified.

 printf(foo);       */ bad */
 printf("%s", foo); */ good */

Lengths

When counting, lengths should be unsigned (unsigned int, not int). Unsigned numbers can be made to overflow and wrap around to zero. Signed numbers can be made to overflow to negative values. A large positive unsigned value is converted to a negative signed value. A common code assumption which can fail with signed values:

 if (length < maxlength) {...}

A negative length passed to a 'secure' string function (strncpy) normally disables the check.

Network Data

Data received from the network should normally be read into byte arrays not char arrays. Char's are by default signed, which is not what is wanted.

Function Calls

Not all functions succeed, check your called functions for return values! (This is done all over the codebase).

Files

  • If you are going to open a file which is to be later trusted, open it with exclusive flags.