Coding Security

wzdftpd is a crucial piece of software accessible remotely to users who cannot be trusted. Therefore the security of wzdftpd is of extreme importance. Developers must ensure that their code is written carefully, with security in mind at every step.

Buffer Overflows

Wikipedia has a very helpful (and long) page describing this form of programming flaw. This is the most important mistake you should avoid at all costs. Almost all remote exploits are due to buffer overflow programming mistakes. Make sure you understand what a buffer overflow flaw is before commencing work on coding for wzdftpd.

An example of bad code that would result in a server being hacked:

/* overflow.c - demonstrates a buffer overflow */

#include <stdio.h>
#include <string.h>

int main(int argc, char *argv[])
{
  char buffer[10];
  if (argc < 2)
  {
    fprintf(stderr, "USAGE: %s string\n", argv[0]);
    return 1;
  }
  strcpy(buffer, argv[1]);
  return 0;
}

An example of the same program, but fixed so that the flaw no longer exists:

/* better.c - demonstrates one method of fixing the problem */

#include <stdio.h>
#include <string.h>

int main(int argc, char *argv[])
{
  char buffer[10];
  if (argc < 2)
  {
    fprintf(stderr, "USAGE: %s string\n", argv[0]);
    return 1;
  }
  strncpy(buffer, argv[1], sizeof(buffer));
  buffer[sizeof(buffer) - 1] = '\0';  /* explicitly write a string terminator */
  return 0;
}

So remember, be very careful with code that messes around with chunks of memory and strings!

Race Conditions

Because wzdftpd is multi-threaded, it can also suffer from timing flaws. Wikipedia has a detailed page explaining what a race condition is, and how to prevent them from occurring.

Secure Input and Output Handling

wzdftpd processes data submitted by untrusted users, and therefore this data should always be treated as unsafe (until sanitized). As an example, SQL injection attacks are very common and highly dangerous, allowing an attacker to read sensitive information such as passwords, perform privilege escalation, bypass passwords, etc. Another problem that affects FTP servers in particular is the processing and filtering file and path names requested by the user. This is often referred to as directory traversal and can allow attackers to read/modify files on the entire system that they shouldn't normally have access to.

An example of a PHP script which has a directory traversal flaw:

<?php
$template = 'blue.php';
if ( is_set( $_COOKIE['TEMPLATE'] ) )
   $template = $_COOKIE['TEMPLATE'];
include ( "/home/users/phpguru/templates/" . $template );
?>

If a user submits this request:

GET /vulnerable.php HTTP/1.0
Cookie: TEMPLATE=../../../../../../../../../etc/passwd

They will end up receiving the password database for the entire server, similar to:

HTTP/1.0 200 OK
Content-Type: text/html
Server: Apache

root:fi3sED95ibqR6:0:1:System Operator:/:/bin/ksh 
daemon:*:1:1::/tmp: 
phpguru:f8fk3j1OIf31.:182:100:Developer:/home/users/phpguru/:/bin/csh

Wikipedia has a page dedicated to this topic, which is worth reading.

Logic Flaws

When coding wzdftpd, it can be quite easy to get mixed up and introduce logic flaws into the permissions and access control mechanisms within wzdftpd. The best solution to overcoming logic flaws is to write good documentation and code comments on how wzdftpd works. If you change the flow of a function, go through each possible path that the function can be executed in and make sure each path is secure. This means you need to analyze each branch statement (if/case/?/for/do/while/...etc) and make sure that the flow of logic does not introduced security problems.

Some examples of this issue include this MySQL vulnerability which meant that attackers could login by guessing just the first letter of the real password:

while (*scrambled)
{
if (*scrambled++ != (char) (*to++ ^ extra))
return 1; /* Wrong password */
}

Another MySQL vulnerability allowed users to login to any account with a NULL length password (bypassing all password security). There is an accompanying whitepaper on this issue which may be of interest.

  /*
    Old clients send null-terminated string as password; new clients send
    the size (1 byte) + string (not null-terminated). Hence in case of
empty
    password both send '\0'.
  */
  uint passwd_len= thd->client_capabilities & CLIENT_SECURE_CONNECTION ?
    *passwd++ : strlen(passwd);

Typing Errors

There was an infamous incident with the Linux kernel development where an attacker almost managed to introduce a backdoor into the kernel code. The modification which would have made this possible was just one single character in the source code. The character which would have created the backdoor can be seen in the following line of code:

if ((options == (__WCLONE|__WALL)) && (current->uid '''=''' 0))

One equals (=) character was removed from the line of code, which has the result of setting the current user ID to 0 instead of just comparing it. This should serve as a good lesson on checking your code very carefully, as these sort of flaws are extremely difficult to detect.

Slashdot readers responded to this incident and it is an interesting and informative read.

---

See also: Old Mediawiki page on coding security, Security exploit articles on Wikipedia