What PScan does:
printf
style functions. e.g.:
sprintf(buffer, variable);
Bad! Possible security breach!
sprintf(buffer, "%s", variable);
Ok
What PScan does not do:
Analyzing and correcting the security breaches is up to the programmer.
Links
pscan-1.1.tar.gz distribution tar file
README main documentation
COPYING license
pscan.h C header file
pscan.c C source
scanner.l LEX rules
test.c Sample C source file
wu-ftpd.pscan Application-specific
problem function definition file
[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.