Bug 3584 - base64 function does not pad output correctly
base64 function does not pad output correctly
Status: CLOSED FIXED
Product: rsync
Classification: Unclassified
Component: core
2.6.6
All All
: P3 normal
: ---
Assigned To: Wayne Davison
Rsync QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-06 05:19 UTC by Mathijs Brands
Modified: 2006-03-12 02:56 UTC (History)
0 users

See Also:


Attachments
Patch for base64 in authenticate.c (368 bytes, patch)
2006-03-06 05:21 UTC, Mathijs Brands
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mathijs Brands 2006-03-06 05:19:13 UTC
The base64 function in authenticate.c does not correcly pad the output data. This can easily be seen by having it encode N bytes of data, where N is -not- a multiple of 3. For instance:

base64("123")  == "MTIz"   (correct value: "MTIz")
base64("1234") == "MTIzNA" (correct value: "MTIzNA==")

Because of this bug, HTTP Basic authentication may not work correctly, depending on the length of the username and password.

The patch below fixes this problem.

----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----8<----

--- rsync-2.6.6/authenticate.c-orig     2006-03-06 11:10:23.000000000 +0100
+++ rsync-2.6.6/authenticate.c  2006-03-06 11:10:54.000000000 +0100
@@ -49,6 +49,9 @@
                }
                out[i] = b64[idx];
        }
+
+       while ((i % 4) > 0)
+               out[i++] = '=';
 }

 /* Generate a challenge buffer and return it base64-encoded. */
Comment 1 Mathijs Brands 2006-03-06 05:21:28 UTC
Created attachment 1775 [details]
Patch for base64 in authenticate.c
Comment 2 Wayne Davison 2006-03-06 12:33:50 UTC
Your change would also affect the password authentication that a daemon rsync performs, making daemons/clients incompatible with older clients/daemons.  Also, your code failed to null-terminate the padded value.

I've checked-in an improved version that lets the caller choose if they want padding or not.  This ensures that only the Proxy-Authentication header is affected by this change.

Thanks for your help.