Created attachment 35313 [details] poc file The htdigest tool has a stack buffer overflow bug if you pass it an input file with a long line. I'll attach a sample file (it simply consists of 766 times "a".) Usually I'd report this as a security vulnerability, but as it only affects a rarely used command line tool I thought I can skip that. This bug was found with afl. When compiling with address sanitizer and passing that file (and any realm/username) it will show the stack overflow: ==4285==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffe5aa62f70 at pc 0x000000509cb6 bp 0x7ffe5aa623f0 sp 0x7ffe5aa623e8 WRITE of size 1 at 0x7ffe5aa62f70 thread T0 #0 0x509cb5 in getword /f/apache/httpd-2.4.27/support/htdigest.c:83:17 #1 0x509cb5 in main /f/apache/httpd-2.4.27/support/htdigest.c:264 #2 0x7ff1e92cc520 in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.25-r4/work/glibc-2.25/csu/../csu/libc-start.c:295 #3 0x419fa9 in _start (/r/apache/htdigest+0x419fa9) Address 0x7ffe5aa62f70 is located in stack of thread T0 at offset 2928 in frame #0 0x5087af in main /f/apache/httpd-2.4.27/support/htdigest.c:187 This frame has 13 object(s): [32, 33) 'ch.i' [48, 52) 'argc.addr' [64, 72) 'argv.addr' [96, 104) 'f' [128, 132) 'rv' [144, 164) 'tn' [208, 216) 'dirname' [240, 496) 'user' [560, 816) 'realm' [880, 1648) 'line' [1776, 2544) 'l' [2672, 2928) 'w' <== Memory access at offset 2928 overflows this variable [2992, 3248) 'x'
Created attachment 35314 [details] smaller poc Alternative poc: 255 chars "a" and a newline
Hi Hanno, thanks a lot for the report. The following patch seems to work for me: ./support/htdigest poc try elukey The following line is longer than the maximum allowed (256): aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa I am not familiar with the htdigest.c code so I'll need to take a deeper look. Will wait for other people's comments too :) Luca Patch: Index: support/htdigest.c =================================================================== --- support/htdigest.c (revision 1807869) +++ support/htdigest.c (working copy) @@ -256,6 +256,11 @@ found = 0; while (!(get_line(line, sizeof(line), f))) { + if (strlen(line) >= MAX_STRING_LEN) { + apr_file_printf(errfile, "The following line is longer than the " + "maximum allowed (%i): %s", MAX_STRING_LEN, line); + cleanup_tempfile_and_exit(1); + } if (found || (line[0] == '#') || (!line[0])) { putline(tfp, line); continue;
if (strlen(line) >= MAX_STRING_LEN) should probably be if (strlen(line) > MAX_STRING_LEN).
(In reply to Luca Toscano from comment #3) > if (strlen(line) >= MAX_STRING_LEN) should probably be if (strlen(line) > > MAX_STRING_LEN). No, it needs to be >=, else the smaller poc will still trigger an overflow (I assume it needs to consider a trailing null pointer). Probably it should me MAX_STRING_LEN-1 in the error message.
Created attachment 35315 [details] htdigest patch
I added a new patch that is probably better, since the problem is getword() and the MAX_STRING_LEN applies to it (so anything split by a ':'), not to the total line length (that can be up to 3 * MAX_STRING_LEN). The attached patch should emit an error and avoid the overflow. Still didn't find a ton of time to test it, will do it in the following days.
Re-compiled trunk with -fsanitize=address and no overflow is registered: $ ./support/htdigest poc try elukey The following line contains a string longer than the allowed maximum size (255): aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa $ ./support/htdigest smallerpoc try elukey The following line contains a string longer than the allowed maximum size (255): aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
I just noted there's already been a patch for an issue that sounds very similar: https://svn.apache.org/viewvc?view=revision&revision=1475878 Even got a CVE: CVE-2013-1862 (I strongly suggest to add some of the poc files as test cases in order to avoid future reappearing of the same bug type)
(In reply to hanno from comment #8) > I just noted there's already been a patch for an issue that sounds very > similar: > https://svn.apache.org/viewvc?view=revision&revision=1475878 Yep, different function (getline). > Even got a CVE: CVE-2013-1862 I am not seeing anything related to htdigest in https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-1862, where did you find the mention of the CVE? > (I strongly suggest to add some of the poc files as test cases in order to > avoid future reappearing of the same bug type) Makes sense, will try to see what I can do in the testing framework.
(In reply to Luca Toscano from comment #9) > I am not seeing anything related to htdigest in > https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-1862, where did you > find the mention of the CVE? Sorry, that was incorrect. I got confused by this bug report: https://bugs.mageia.org/show_bug.cgi?id=10097 It references both the CVE and this bug, but it seems they are unrelated and this bug report just discusses fixes for multiple unrelated security bugs.
Proposed the fix in http://svn.apache.org/r1808008 (trunk), waiting for the review of other members of the httpd dev community.
Change backported to 2.4.x with http://svn.apache.org/r1808853, will be part of the new release (2.4.28)