Re: Fix for DoS in LIST command with GNU ls

This is certainly one solution to the immediate problem.  However, if there is
an OS out there which is not using GNU ls, but instead has an ls which supports
a -w argument which achieves a different effect, you'd be hurting the user by
denying the -w option.

I'd recommend wrapping the call to sanitise_ls_args() in an "#if defined" so
that it is only called if the GNU ls is being used (or another ls with the
same problem).


Long term, it would be helpful to understand the root cause of the problem,
and fix that, so that the "-w" switch is available for GNU users if desired.

For example, is this just a problem with wu-ftpd, or is it really a problem
with the GNU ls program ?  If I were to say, telnet into a GNU host and run
ls in the same way as described for wu-ftpd, can I achieve a DoS ?  If the
problem lies with wu-ftpd, is there some similar attack which can be performed
using a different switch with a non-GNU ls (or GNU ls, for that matter) ?

If the problem lies with GNU, then it would be best fixed there.  If the
problem lies with wu-ftpd, then it would be best to find the root cause and
fix that rather then simply deny the "-w" option, in case denying "-w" does
not block all such attacks (for example if a different ls is used).

Sadly, my "real" workload doesn't allow me the time to investigate the issue
further (nor do I run ftpd under Linux).  But I certainly appreciate your
bringing the issue and workaround to the attention of the wu-ftpd developer
community.

Thanks !

Cheers, Bob

On Tue, Feb 22, 2005 at 09:44:03PM +0000, Chris Butler wrote:
> Hi,
> 
> I've had a bug reported via the Debian Bug Tracking System about a DoS
> attack against wu-ftpd, that exploits the --width argument to GNU 'ls'.
> By repeatedly giving it a very large argument, an attacker can cause ls
> to exhaust the system memory.
> 
> The attached patch adds a function to ftpcmd.y, called sanitise_ls_args,
> which removes the -w argument and its argument from the arguments to the
> ls command.
> 
> -- 
> Chris Butler    <[email protected]>
>  Debian WU-FTPD package maintainer
>    GnuPG Key ID: 1024D/D097A261

> Index: ftpcmd.y
> ===================================================================
> --- ftpcmd.y	(revision 33)
> +++ ftpcmd.y	(working copy)
> @@ -395,15 +395,22 @@
>  	    }
>  	}
>      | LIST check_login SP pathname CRLF	{
> +			char *ls_args;
> +		
>  	    if (log_commands)
>  		syslog(LOG_INFO, "LIST %s", CHECKNULL($4));
>  	    if ($2 && $4 != NULL && !restrict_list_check($4)) {
>  		retrieve_is_data = 0;
>  #ifndef INTERNAL_LS
> +		ls_args = sanitise_ls_args($4);
> +
>  		if (anonymous && dolreplies)
> -		    retrieve(ls_long, $4);
> +		    retrieve(ls_long, ls_args);
>  		else
> -		    retrieve(ls_short, $4);
> +		    retrieve(ls_short, ls_args);
> +
> +		if (ls_args != NULL)
> +			free(ls_args);
>  #else
>  		ls($4, 0);
>  #endif
> @@ -1937,3 +1944,38 @@
>      (void) fflush(stdout);
>      reply(214, "");
>  }
> +
> +char *sanitise_ls_args(char *inp)
> +{
> +		/* sanitise arguments to ls, to avoid -w DoS with GNU ls */
> +    char *ls_args, *inp_p, *ls_args_p;
> +
> +		if (inp == NULL)
> +			return NULL;
> +								
> +		ls_args = malloc(strlen(inp) + 1);
> +    inp_p = inp;
> +    ls_args_p = ls_args;
> +								
> +    while(*inp_p != '\0') {
> +      if (strncasecmp(inp_p,"-w",2) == 0) {
> +  	    /* skip -w and its argument */
> +        inp_p += 2;
> +        for (;*inp_p != '\0' && isspace(*inp_p);inp_p++);
> +        for (;*inp_p != '\0' && !isspace(*inp_p);inp_p++);
> +      } else if (strncasecmp(inp_p,"--width",7) == 0) {
> +  	    /* same with the long option */
> +        inp_p += 7;
> +        for (;*inp_p != '\0' && isspace(*inp_p);inp_p++);
> +        for (;*inp_p != '\0' && !isspace(*inp_p);inp_p++);
> +      } else {
> +        *ls_args_p = *inp_p;
> +				++ls_args_p;
> +				++inp_p;
> +			}
> +    }
> +
> +		*ls_args_p = '\0';
> +    return ls_args;
> +}
> +
> Index: proto.h
> ===================================================================
> --- proto.h	(revision 33)
> +++ proto.h	(working copy)
> @@ -135,6 +135,7 @@
>  void alias(char *s);
>  void cdpath(void);
>  void print_groups(void);
> +char *sanitise_ls_args(char *);
>  
>  /*
>     ** ftpd.c

-- 
Bob Luckin      [email protected]      "Coder, adapt; FTP Ada, redo C"