Introduction

This article should serve as an checklist for developers to verify their code quickly for well-known security problems.

First of all, you just need to verify that the code which processes data from beyond your programs domain, that is, direct user-input, reading from non-system files, reading data from the network, processing binary data (like JPEG images), receiving results from database/directory/... servers, etc.

Code that should run with higher privileges like system daemons or setuid applications need special care, because they are representing a high risk for system security. This kind of code should always be reviewed by the SUSE security team.

Note: The purpose of this article is not to be complete and to describe special cases, but to be used easily and quickly verify your code.

C and C++ Code

Check for compiler warnings

Be sure to use at least -Wall -Wmissing-declarations -Wmissing-prototypes -Wredundant-decls -Wshadow -Wstrict-prototypes -Wformat=2 as compiler flags to gcc to get notified for some of the potential errors presented below.

Check for: system(), popen()

Both calls use the UNIX command shell to fulfill their work. Therefore, untrusted input will lead to a direct breach of system security (shell commands can be executed with metachars, or command line options can be added or changed). It is better to avoid the usage of system(3) and replace it with a combination of fork(2) and exec(2) (without exec'ing the shell of course). popen() can also be replaced with a more secure version. For an example implementation have a look at http://www.suse.de/~thomas/projects/secproglib/index.html (function: s_popen()) or HXproc_* from libHX (the latter is shipped in opeSUSE). Besides the possible security problems those functions can cause, they also have a high overhead and make your code slow.

Similar problems can arise when programs are executed (not via the shell, but directly) that support escape sequences to execute other commands. For example, '!' allows executing shell commands in less or some mail clients (MUA) allow escape sequences to execute external programs. To not become a victim of this kind of feature, read the program's documentation.

Check for: strcpy(), strcat(), sprintf(), scanf(), gets(), ...

These functions do not check for an upper limit of the destination buffer and copy all bytes from the source buffer until '\0' (ASCII NUL) is found. A typical way to handle this behavior is to allocate enough memory space for the destination buffer before copying the data. Here lies another problem, a buffer too big can be allocated, which leads to a denial-of-service situation, or integer overflows can cause a buffer too small to be allocated, which in turn leads to a buffer overflow — more about this later. The function gets() can not be made secure, it can just be replaced.

Check for: strncpy(), strncat(), snprintf(), ...

To avoid the problem of not having an upper limit parameter for data copying, a set of new and equal functions are provided to the programmer. The only pitfalls that can cause trouble here is the wrong size of the parameter and unexpected result of arithmetical operations. For the first problem, always make sure that the size parameter represents the size of the destination buffer and not more (using the size of the source buffer is not uncommon but renders the upper limit checking useless). The second issue is more tricky. The size variable should be of type size_t (this is the common type for such parameters, it is also portable, but better read the documentation), additionally make sure all operations that change this variable are of the same type — be type-safe and do not overflow, or become too big. For example:

size_t len;
...
strncpy(dst, src, len - 1);
...

When len is 0 then len - 1 will be a high value and the dst buffer will be overflown.

Since the use of strncat for example can become awkward, as in:

char buf[1234];
size_t len = sizeof(buf);
buf[len] = '\0';
strncpy(buf, "Hello", len - 1);
strncat(buf, " World", len - 1 - strlen(buf));

BSD has introduced functions like strlcpy that are simpler to use,

char buf[1234];
size_t len = sizeof(buf);
strlcpy(buf, "Hello", len);
strlcat(buf, " World", len);

The functions are available in the previously-mentioned libHX as HX_strlcat, for example.

Check for: malloc() & co.

When you allocate memory and external parameters are part of the size, then you should make sure you do not allocate too much memory (denial-of-service) by setting an upper limit which makes sense for your application. You should also make sure that no integer overflows or other arithmetical issues happen. This could lead to overflows on the heap of your process and allow an attacker to execute arbitrary code. It is best to only use unsigned integers (like type size_t), check for an upper and lower limit, and to verify if a possible operation with the untrusted integer(s) lead to an integer overflow. A typical test looks like this:

size_t size;

if(size*sizeof(XRefEntry)/sizeof(XRefEntry) != size)
{
  error(-1, "Invalid 'size' inside xref table.");
}

entries = malloc(size * sizeof(XRefEntry));
...

Check for: wrong casts

There is no need to cast the result of library functions that return void *; it makes your code hard to read, adds no value, and can hide a bug if you don't have a valid prototype in scope. See http://www.cpax.org.uk/prg/writings/casting.php and http://c-faq.com/malloc/mallocnocast.html .

Check for: loops iterating over arrays/strings

Buffer and integer overflows often occur in loops, make sure the loop also stops if an upper/lower limit of an array or integer variable is reached.

Check for: variable parameter lists

A relatively new kind of problem are so called format string bugs. They occur whenever functions that allow a variable list of parameters (like printf()) and untrusted data is directly used as format string and not as format argument. See the following example to make this clear:

wrong: snprintf(buf, sizeof(buf), user_input); // what was probably intended: strlcpy(buf, user_input, sizeof(buf));
right: snprintf(buf, sizeof(buf), "%s", user_input);

Check for: file operations

Handling files can be problematic depending on the context. Each subsection will cover one context.

For temporary files mkstemp() should be used.

Creating files: permissions

The permissions of a file are important. Not everyone should be able to read or write from/to a file. For example, think of a file that contains confidential information and is created with permissions 0644. Another peculiarity is to take care of is the base of the integer representing the permissions, it is octal. Therefore 644 (decimal) is different from 0644 (octal). Using umask() at the beginning of the code reduces the risk of creating files with the wrong permission. For example: umask(077)

Creating files: owner

If you want to create a file, and another file with the same name does already exist in the path, you should check if it is a regular file and that it is owned by the same user before operating on it. This is especially important for processes that run with higher privileges. Use open() and fstat() to get the file information otherwise your code may become vulnerable to so called race conditions.

Creating files: symbolic links

It is sometime problematic if your code follows links on the filesystem from one file to another. File owner checks should be sufficient here too.

Processes with higher privileges

If your process runs with higher privileges and operates with files of another user, it is wise to change the UID and GID to that user instead of keeping the higher ones. This can be done in an extra thread/process.

Check for: process environment

If you want to pass secret information (like a password) to another process do not use the process environment for it — preferably use an anonymous pipe (or alike, such as a socket). Depending on the operating system, the process environment can be examined by every user or just by a user with the same UID. Another fact to take care of is that the environment cannot be trusted. The caller of a process can change a variable to a arbitrary value.

Check for: dropping privileges

The order of the system calls is important. Use:

initgroups()
setgid()
setuid()

And always verify the return value.

Check for: handling sensible information

Sensible data that is written to disk should be encrypted with a well-known state-of-the-art cryptographic algorithm. If the information is kept in plaintext (for example in config files), only root should be allowed to access the file and no one else. Whenever files that contain sensible information in plaintext (for example in temporary files) are removed from the disk, it is always good to overwrite the file with random data before calling remove(), unlink() etc. Removing information from the memory of a process is a bit more complicated, because compilers like to optimize the code and remove the needed instructions. Therefore, crypto keys and passwords should not be kept in stack buffer, but in heap buffers, and the content of the buffer should be overwritten when the key or password is not used anymore, like "memset(buf, 0, buf_len)". This is especially a problem with languages that use a garbage collector or/and immutable strings, like Java. In the case of Java, an array can be used instead of a string.

Additionally swapping can be turned off using mlock() and core dumps can be avoided with setrlimit() to avoid writing process memory to disk.

This solution can not guarantee that information leaks somewhere and somehow else, but they are relatively easy to use to provide a fix for a marginal problem.

Java

Java is vulnerable to integer overflows (no exception thrown), and handle files insecurely. (For details, have a look at the section about C and C++.) Additionally, Java uses so called native code, which is often written in low-level programming languages like C, therefore Java can also be vulnerable to buffer overflows or format string bugs. These kind of issues already happened while processing JPEG ICC profiles.

Perl

See http://www.linux-knowledge-portal.org/en/content.php?&content/security/secprog8.html

Unix Shell Script

See http://www.linux-knowledge-portal.org/en/content.php?&content/security/secprog7.html

Using Cryptography

use well-known libraries and up-to-date algorithms (like openssl and AES for example)
use large enough keys (>=2048 bit for RSA, >=128 bit for block ciphers)
verify certificates if you use public key crypto (SSL, PKI)
use getentropy(3) when entropy is needed for keys,session IDs, IVs and such

SDB:Secure coding checklist: C and C++

Contents