You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@apr.apache.org by bu...@apache.org on 2008/08/23 15:12:27 UTC

DO NOT REPLY [Bug 45679] New: SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

https://issues.apache.org/bugzilla/show_bug.cgi?id=45679

           Summary: SHA1 passwords starting with {SHA} don't work and cause
                    a minor buffer overrun
           Product: APR
           Version: HEAD
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: APR-util
        AssignedTo: bugs@apr.apache.org
        ReportedBy: bnoordhuis@gmail.com


Created an attachment (id=22477)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=22477)
Teeny weeny patch

The summary says it all really, but for posterity's sake:

htpasswd -s -c /path/to/htpasswd bnoordhuis
New password: {SHA}foo
Re-type new password: {SHA}foo

Due to erroneous code in apr_sha1_base64() the generated password hash will
never match the original password. And because the pointer to the plain-text
password is bumped but the length variable isn't, a buffer overrun of several
bytes happens when generating the hash. I doubt it is exploitable (the stack
layout doesn't seem to favor a potential attacker) but I'm no expert on the
matter.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #5 from Bojan Smojver <bo...@rexursive.com>  2008-08-25 05:28:16 PST ---
BTW, this code appears to be all the way from Apache 1.3.x.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #2 from Ben Noordhuis <bn...@gmail.com>  2008-08-25 02:54:31 PST ---
> Does the above work for you?

Yes and no. It fixes the out-of-bounds reads but passwords starting with
'{SHA}' still don't work. I do believe the pointer bump needs to go - it is not
the .htpasswd hash you are chopping bits off, it is the plain-text password the
user entered.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #7 from Bojan Smojver <bo...@rexursive.com>  2008-08-26 00:27:17 PST ---
That's easy. Just send an e-mail to APR devel list, describe the problem and
point out that you're not sure if the prefix stripping needs to be kept for
some historical reason or not. I honestly don't know - I haven't been involved
with APR that long - but there are people on the list that are bound to know.

BTW, the code of Apache 1.3.x also needs to be fixed.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #4 from Bojan Smojver <bo...@rexursive.com>  2008-08-25 04:20:54 PST ---
But I also do see your point about "{SHA}" prefix actually being part of the
final hash, not the clear password passed in (this is how OpenLDAP works, for
instance). To be honest, I don't know why the exception to ignore "{SHA}"
prefix in clear password would be made...

Maybe long, long time ago someone wanted to identify SHA1 password hashing by
passing that prefix in front of clear passwords or something. No idea...


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #3 from Bojan Smojver <bo...@rexursive.com>  2008-08-25 04:04:07 PST ---
Isn't the point here that "{SHA}" prefix isn't actually part of the password?

The way I'm reading the code is that "clear" can be either {SHA}password or
just password and base64 encoded value "out" should be the same. No?

Maybe you have hashes of {SHA}password stored in your .htpasswd file (instead
of just password), so the patch gives different values of the hash, because it
ignores the "{SHA}" prefix? In other words, if you regenerate .htpasswd from
scratch, does it work?


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679





--- Comment #6 from Ben Noordhuis <bn...@gmail.com>  2008-08-26 00:15:58 PST ---
> Maybe long, long time ago someone wanted to
> identify SHA1 password hashing by passing that
> prefix in front of clear passwords or something.
> No idea...

That thought struck me as well but. grepping through the source, I don't find
anything that would require - let alone justify - stripping the prefix (see
below). So what do we do now? I still think those three lines have to go.

$ find . -name '*.c' | xargs egrep '(apr_sha1_base64|ap_hack_apr_sha1_base64)'
./support/htpasswd.c:        apr_sha1_base64(pw,strlen(pw),cpw);
./support/htdbm.c:           
apr_sha1_base64(htdbm->userpass,strlen(htdbm->userpass),cpw);
./srclib/apr-util/crypto/apr_md5.c: * crypt() (if available) or
apr_md5_encode() or apr_sha1_base64(), depending
./srclib/apr-util/crypto/apr_md5.c:        apr_sha1_base64(passwd,
(int)strlen(passwd), sample);
./srclib/apr-util/crypto/apr_sha1.c: *   apr_sha1_base64(const char *clear, int
len, char *out);
./srclib/apr-util/crypto/apr_sha1.c:APU_DECLARE(void) apr_sha1_base64(const
char *clear, int len, char *out)
./srclib/apr-util/exports.c:const void *ap_hack_apr_sha1_base64 = (const void
*)apr_sha1_base64;
./srclib/apr-util/test/testpass.c:    apr_sha1_base64(pass, (int)strlen(pass),
hash);
./server/exports.c:const void *ap_hack_apr_sha1_base64 = (const void
*)apr_sha1_base64;


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679


Bojan Smojver <bo...@rexursive.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bojan@rexursive.com




--- Comment #1 from Bojan Smojver <bo...@rexursive.com>  2008-08-25 01:25:21 PST ---
I don't think this patch is correct. I think we should actually do this (note
that there is a tab in the first patched line, hence the diff):

Index: crypto/apr_sha1.c
===================================================================
--- crypto/apr_sha1.c   (revision 685796)
+++ crypto/apr_sha1.c   (working copy)
@@ -352,7 +352,8 @@
     apr_byte_t digest[APR_SHA1_DIGESTSIZE];

     if (strncmp(clear, APR_SHA1PW_ID, APR_SHA1PW_IDLEN) == 0) {
-       clear += APR_SHA1PW_IDLEN;
+        clear += APR_SHA1PW_IDLEN;
+        len -= APR_SHA1PW_IDLEN;
     }

     apr_sha1_init(&context);

Does the above work for you?


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org


DO NOT REPLY [Bug 45679] SHA1 passwords starting with {SHA} don't work and cause a minor buffer overrun

Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=45679


Bojan Smojver <bo...@rexursive.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #8 from Bojan Smojver <bo...@rexursive.com>  2009-01-04 18:21:02 PST ---
Nobody objected to this, so it's now committed to the trunk, 1.3.x and 1.4.x.

Thanks for the patch!


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@apr.apache.org
For additional commands, e-mail: bugs-help@apr.apache.org