Lighttpd

Please use unified diffs (diff -u) for patches; and could you also add a test case in test_base64.c which fails on your platform?

What operating system are you on and which compiler are you using?

Your patch would result in additional signed/unsigned warnings/errors from other compilers.

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.

Using unsigned char would also be better than explicit signed char, but would also require additional casting.

For your system, which must be treating char as unsigned char, this might be the simplest fix:

--- 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 */
                }

You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'

For your system, which must be treating char as unsigned char, this might be the simplest fix:
[...]

You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'

Is this expectation for lighttpd or just for this function??
This is the most important point to me.
If for all of lighttpd, then your build system must address this is.

And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.
I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.

codehero wrote:

For your system, which must be treating char as unsigned char, this might be the simplest fix:
[...]

You should check your compiler options and consider compiling lighttpd with explicit compiler switch to treat 'char' as 'signed char'

Is this expectation for lighttpd or just for this function??
This is the most important point to me.
If for all of lighttpd, then your build system must address this is.

I agree. And imho lighttpd should handle char == unsigned char.

And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.
I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.

"leaking memory" - you got any valgrind traces for that?

I agree. And imho lighttpd should handle char == unsigned char.

Sounds good to me; hope you guys can decide on it.

And to be honest, I don't have the time to further pursue a solution for the issue; I have to move on.
I am trying to get something delivered and mod_authn_ldap is leaking memory like crazy.

"leaking memory" - you got any valgrind traces for that?

I opened up bug https://redmine.lighttpd.net/issues/2849

When building with the following patch on x86_64 and compiling with -funsigned-char, make check passes, but without this patch, make check 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'.

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:

bug introduced in lighttpd 1.4.46 in commit 7aff5046