bugtraq - Re: local buffer overflow in htpasswd for apache 1.3.31 not fixed in .33?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.58.0410292155510.4658@bello.engert.org>
Date: Fri, 29 Oct 2004 23:34:08 +0200 (CEST)
From: Michael Engert <michi@...lo.engert.org>
To: Larry Cashdollar <lwc@...id.ath.cx>,
	Luiz Fernando <foxtrott@...wsecurity.org>
Cc: bugtraq@...urityfocus.com
Subject: Re: local buffer overflow in htpasswd for apache 1.3.31 not fixed
 in .33?


Hello,

I can confirm the buffer overflow in htpasswd of apache 1.3.33, for which 
Luiz Fernando has written a PoC. ...

On Fri, 29 Oct 2004, Larry Cashdollar wrote:
> This was posted on the full-disclosure list sept 16 2004 by
> Luiz Fernando.
> 
> http://archives.neohapsis.com/archives/fulldisclosure/2004-09/0547.html
[...]
> It is still vulnerable.

But Larrys patch "fixes" a lot of peaces of code, which aren't 
vulnerable in my oppinion. A closer look shows, that the calls to strcpy 
are protected by if-statements, which prevent a exploitation. Its just one 
place where a closing brace ('}') is at the wrong position. ;-)

So, instead of this ...

> root@...choy:~/tes/apache_1.3.33/src/support# diff -uN  htpasswd.orig.c
> htpasswd.c
> --- htpasswd.orig.c     2004-10-28 18:20:13.000000000 -0400
> +++ htpasswd.c  2004-10-28 18:17:25.000000000 -0400
> @@ -202,9 +202,9 @@
>         ap_cpystrn(record, "resultant record too long", (rlen - 1));
>         return ERR_OVERFLOW;
>      }
> -    strcpy(record, user);
> +    strncpy(record, user,MAX_STRING_LEN - 1);
>      strcat(record, ":");
> -    strcat(record, cpw);
> +    strncat(record, cpw,MAX_STRING_LEN - 1);
>      return 0;
>  }
> 
> @@ -410,14 +410,14 @@
>             fprintf(stderr, "%s: filename too long\n", argv[0]);
>             return ERR_OVERFLOW;
>         }
> -       strcpy(pwfilename, argv[i]);
> +       strncpy(pwfilename, argv[i], MAX_STRING_LEN-1);
>         if (strlen(argv[i + 1]) > (sizeof(user) - 1)) {
>             fprintf(stderr, "%s: username too long (>%lu)\n", argv[0],
>                     (unsigned long)(sizeof(user) - 1));
>             return ERR_OVERFLOW;
>         }
>      }
> -    strcpy(user, argv[i + 1]);
> +    strncpy(user, argv[i + 1],MAX_STRING_LEN-1);
>      if ((arg = strchr(user, ':')) != NULL) {
>         fprintf(stderr, "%s: username contains illegal character
> '%c'\n",
>                 argv[0], *arg);
> @@ -429,7 +429,7 @@
>                     (unsigned long)(sizeof(password) - 1));
>             return ERR_OVERFLOW;
>         }
> -       strcpy(password, argv[i + 2]);
> +       strncpy(password, argv[i + 2],MAX_STRING_LEN - 1 );
>      }
> 
>  #ifdef WIN32
> @@ -553,7 +553,7 @@
>                 putline(ftemp, line);
>                 continue;
>             }
> -           strcpy(scratch, line);
> +           strncpy(scratch, line,MAX_STRING_LEN -1);
>             /*
>              * See if this is our user.
>              */

... I suggest the following shorter one, which will give us also a correct 
error message instead of eventually filling htpasswd with "short" entries:

| --- src/support/htpasswd.c.orig	Fri Feb 20 23:02:24 2004
| +++ src/support/htpasswd.c	Fri Oct 29 21:13:36 2004
| @@ -411,11 +411,11 @@
| 	    return ERR_OVERFLOW;
| 	}
| 	strcpy(pwfilename, argv[i]);
|-	if (strlen(argv[i + 1]) > (sizeof(user) - 1)) {
|-	    fprintf(stderr, "%s: username too long (>%lu)\n", argv[0],
|-		    (unsigned long)(sizeof(user) - 1));
|-	    return ERR_OVERFLOW;
|-	}
|+    }
|+    if (strlen(argv[i + 1]) > (sizeof(user) - 1)) {
|+	fprintf(stderr, "%s: username too long (>%lu)\n", argv[0],
|+		(unsigned long)(sizeof(user) - 1));
|+	return ERR_OVERFLOW;
|     }
|     strcpy(user, argv[i + 1]);
|     if ((arg = strchr(user, ':')) != NULL) {

This bug exists in 1.3.31, 1.3.32 and 1.3.33. I didn't test other 
versions. As I don't find a entry in the bug database, I reported that bug
to the apache httpd people. The Bug ID is #31975.

Yours, Michi.
----------------------------------------------------------------------------
 Michael Engert                                            michi@...ert.org
 80337 München