https://redmine.lighttpd.net/https://redmine.lighttpd.net/favicon.ico?13667327412017-12-20T09:49:08Zlighty labsLighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112162017-12-20T09:49:08Zstbuehler
<ul></ul><p>Please use unified diffs (<code>diff -u</code>) for patches; and could you also add a test case in <code>test_base64.c</code> which fails on your platform?</p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112172017-12-20T13:38:41Zgstrauss
<ul></ul><p>What operating system are you on and which compiler are you using?</p>
<p>Your patch would result in additional signed/unsigned warnings/errors from other compilers.</p>
<p>A more portable fix might be to modify -1 -2 -3 values in the tables to be out of range for base64, and check for those values for errors, rather than -1 -2 -3.</p>
<p>Using unsigned char would also be better than explicit signed char, but would also require additional casting.</p>
<p>For your system, which must be treating char as unsigned char, this might be the simplest fix:<br /><pre>
--- a/src/base64.c
+++ b/src/base64.c
@@ -72,7 +72,7 @@ unsigned char* buffer_append_base64_decode(buffer *out, const char* in, size_t i
break;
} else if (-2 == ch) {
continue; /* skip character */
- } else if (ch < 0) {
+ } else if (-1 == ch) {
return NULL; /* invalid character, abort */
}
</pre></p>
<p>You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'</p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112182017-12-20T14:13:36Zcodehero
<ul></ul><blockquote>
<p>For your system, which must be treating char as unsigned char, this might be the simplest fix:<br />[...]</p>
<p>You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'</p>
</blockquote>
<p>Is this expectation for lighttpd or just for this function??<br />This is the most important point to me.<br />If for all of lighttpd, then your build system must address this is.</p>
<p>And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.<br />I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.</p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112192017-12-20T14:17:55Zstbuehler
<ul></ul><p>codehero wrote:</p>
<blockquote><blockquote>
<p>For your system, which must be treating char as unsigned char, this might be the simplest fix:<br />[...]</p>
<p>You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'</p>
</blockquote>
<p>Is this expectation for lighttpd or just for this function??<br />This is the most important point to me.<br />If for all of lighttpd, then your build system must address this is.</p>
</blockquote>
<p>I agree. And imho lighttpd should handle <code>char == unsigned char</code>.</p>
<blockquote>
<p>And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.<br />I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.</p>
</blockquote>
<p>"leaking memory" - you got any valgrind traces for that?</p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112202017-12-20T14:36:24Zcodehero
<ul></ul><blockquote>
<p>I agree. And imho lighttpd should handle <code>char == unsigned char</code>.</p>
</blockquote>
<p>Sounds good to me; hope you guys can decide on it.</p>
<blockquote><blockquote>
<p>And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.<br />I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.</p>
</blockquote>
<p>"leaking memory" - you got any valgrind traces for that?</p>
</blockquote>
<p>I opened up bug <a class="external" href="https://redmine.lighttpd.net/issues/2849">https://redmine.lighttpd.net/issues/2849</a></p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112252017-12-21T06:54:07Zgstrauss
<ul></ul><p>When building with the following patch on x86_64 and compiling with -funsigned-char, <code>make check</code> passes, but without this patch, <code>make check</code> fails when building with -funsigned-char. While lighttpd could always use more test cases, nothing else immediately jumps out as depending on 'char' to be 'signed char'.<br /><pre>
diff --git a/src/base64.c b/src/base64.c
index f39dbaa2..3034181a 100644
--- a/src/base64.c
+++ b/src/base64.c
@@ -11,7 +11,7 @@
/* BASE64_STANDARD: "A-Z a-z 0-9 + /" maps to 0-63, pad with "=" */
static const char base64_standard_table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=";
-static const char base64_standard_reverse_table[] = {
+static const signed char base64_standard_reverse_table[] = {
/* 0 1 2 3 4 5 6 7 8 9 A B C D E F */
-1, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, /* 0x00 - 0x0F */
-2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, /* 0x10 - 0x1F */
@@ -25,7 +25,7 @@ static const char base64_standard_reverse_table[] = {
/* BASE64_URL: "A-Z a-z 0-9 - _" maps to 0-63, pad with "." */
static const char base64_url_table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_.";
-static const char base64_url_reverse_table[] = {
+static const signed char base64_url_reverse_table[] = {
/* 0 1 2 3 4 5 6 7 8 9 A B C D E F */
-1, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, /* 0x00 - 0x0F */
-2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, -2, /* 0x10 - 0x1F */
@@ -42,7 +42,7 @@ unsigned char* buffer_append_base64_decode(buffer *out, const char* in, size_t i
size_t out_pos = 0; /* current output character (position) that is decoded. can contain partial result */
unsigned int group = 0; /* how many base64 digits in the current group were decoded already. each group has up to 4 digits */
size_t i;
- const char *base64_reverse_table;
+ const signed char *base64_reverse_table;
switch (charset) {
case BASE64_STANDARD:
</pre></p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112282017-12-21T22:39:54Zgstrauss
<ul></ul><p>bug introduced in lighttpd 1.4.46 in commit <a class="changeset" title="[unittests] consolidate base64 test code consolidate base64 test code use char type for tables ..." href="https://redmine.lighttpd.net/projects/lighttpd/repository/14/revisions/7aff5046ace2e6e776b2a1bd6406acfc727b8ff5">7aff5046</a></p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112292017-12-21T22:50:40Zgstrauss
<ul><li><strong>Status</strong> changed from <i>Patch Pending</i> to <i>Fixed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset <a class="changeset" title="[core] fix base64 decode when char is unsigned (fixes #2848) thx, codehero x-ref: "buffer_app..." href="https://redmine.lighttpd.net/projects/lighttpd/repository/14/revisions/d4083effab0f9bf76528d5c47198b17e7471ed13">d4083effab0f9bf76528d5c47198b17e7471ed13</a>.</p> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=112442018-01-07T04:20:44Zgstrauss
<ul><li><strong>Has duplicate</strong> <i><a class="issue tracker-1 status-10 priority-4 priority-default closed" href="/issues/2851">Bug #2851</a>: mod_auth fails its test suite on PowerPC (big-endian?)</i> added</li></ul> Lighttpd - Bug #2848: buffer_append_base64_decode() broken on compilers where char is assumed unsignedhttps://redmine.lighttpd.net/issues/2848?journal_id=113422018-03-06T02:49:41Zgstrauss
<ul><li><strong>Has duplicate</strong> <i><a class="issue tracker-1 status-10 priority-4 priority-default closed" href="/issues/2873">Bug #2873</a>: Basic Auth method not working</i> added</li></ul>