PScan: A limited problem scanner for C source files

Are you tired of yet more externally exploitable buffer overflows in C programs? Do you want to audit your source for common mistakes? If so, PScan is for you!

What PScan does:

• Scans C source files for problematic uses of printf style functions. e.g.:

  sprintf(buffer, variable); Bad! Possible security breach!
  sprintf(buffer, "%s", variable); Ok

See the Format bugs post to BUGTRAQ for a good explanation of the security problems associated with sprintf(buffer, variable);. These security issues also currently involve being able to bypass stack protection mechanisms like StackGuard. I have submitted a paper to BUGTRAQ describing the issue.

All of these security problems can also occur with any printf-style function. It is simple to fall into the trap of misusing printf and friends, thus, the need for PScan.

What PScan does not do:

• Scan for traditional buffer over-flows.

  You should use a bounds-checking compiler for that.

• Scan for any other mis-use of function parameters.

The functionality given by PScan is limited. Yet it may be useful. I'm not going to claim it's the be-all and end-all of security scanners, but it does one thing, and it does it simply, and reasonably well.

Newer versions of GCC do a better job of scanning source files for problems, but they require the code to be compiled. Pscan is a lot faster, but not as good. As always, there are trade-offs in life.

Analyzing and correcting the security breaches is up to the programmer.

Sample Output

Let's run PScan over an old version of wu-ftpd. The text below is a sample of the output from PScan:

[aland@www pscan]$ ./pscan -p wu-ftpd.pscan ../wu-ftpd-2.6.1/src/*.c
../wu-ftpd-2.6.1/src/ftpd.c:2575 FUNC reply
../wu-ftpd-2.6.1/src/ftpd.c:6277 FUNC syslog
../wu-ftpd-2.6.1/src/ftpd.c:6292 FUNC syslog
../wu-ftpd-2.6.1/src/ftpd.c:6438 FUNC reply
[aland@www pscan]$ 

From the area around line 6277 of ftpd.c, with the problem line emphasized, the code is

6271:    if (debug) {
6273:      char *s = calloc(128 + strlen(remoteident), sizeof(char));
6274:      if (s) {
6275:        int i = ntohs(pasv_addr.sin_port);
6276:        sprintf(s, "PASV port %i assigned to %s", i, remoteident);
6277:        syslog(LOG_DEBUG, s);
6278:        free(s);
6279:      }
6280:    }

So we can see that if the variable debug is set, and the variable remoteident can be set externally (say by an anonymous FTP user), then there may be an exploitable hole in the call to syslog.

If we root around the source a little more, we discover in ftpd.c:

6037:  else if (authenticated)
6038:    sprintf(remoteident, "%s @ %s [%s]",
6039:        authuser, remotehost, remoteaddr);
6040:  else
6041:    sprintf(remoteident, "%s [%s]", remotehost, remoteaddr);

The remotehost variable holds the host name of the remote host which is currently connected. A malicious user may set the DNS hostname to a string which contains carefully constructed formatting codes recognized by the sprintf and syslog functions. This problem may allow him to cause the ftp daemon to core dump, or even for him to gain access to a root shell.

The solution is to correct line 6277 in the source. The suggested replacement line is below, with the changes emphasized

6277:        syslog(LOG_DEBUG, "%s", s);

Trusting user input is a bad thing for any program to do.