In any case, the bind-users is a wrong place to report bugs like this. I would 
ask the OP to report this as an issue in our GitLab, so it does not get lost. 
Also the GL has the template that asks all the questions needed to properly 
diagnose the issue.

Ondrej
--
Ondřej Surý (He/Him)
[email protected]

ADHD brain at work: I sometimes lose track of my inbox. Please feel free to 
send a gentle nudge if you're waiting on a reply!

My working hours and your working hours may be different. Please do not feel 
obligated to reply outside your normal working hours.

> On 1. 7. 2026, at 21:21, Doug Freed <[email protected]> wrote:
> 
> On 7/1/26 08:27, Ismail Ramzi wrote:
>> isc__nm_base64url_to_base64() and isc__nm_base64_to_base64url() in
>> lib/isc/netmgr/http.c index the 256-entry base64url_validation_table[]
>> with a plain char cast straight to size_t:
>>    base64url_validation_table[(size_t)base64url[i]]
>> char is signed on the platforms BIND commonly runs on (x86-64, arm64),
>> so any input byte with the high bit set (0x80-0xFF) is sign-extended
>> before the conversion to size_t.  For example (size_t)(char)0x80 is
>> 0xffffffffffffff80, which is used as an index into a 256-element array,
>> i.e. a wild out-of-bounds read.  Depending on what the read returns the
>> byte is either accepted (and copied into the freshly allocated output,
>> producing corrupt data and a leak) or the process reads unmapped memory
>> and crashes.
>> These are the decoders used to turn the base64url "?dns=" parameter of a
>> DoH GET request into base64, so they are meant to operate on
>> attacker-supplied bytes.  The fix belongs in the decoders themselves and
>> not in the callers: cast each byte to unsigned char before using it as
>> an index, which is the convention already used elsewhere in the tree
>> (e.g. lib/isc/httpd.c).  An unsigned char is always a valid index into
>> the 256-entry table, so the extra size_t cast is no longer needed.
>> A regression case is added to the doh_base64url_to_base64 unit test that
>> feeds a byte with the high bit set; before the fix it reads out of
>> bounds (and wrongly returns a non-NULL result), after the fix the byte
>> is rejected and NULL is returned.
>> ---
>> diff --git a/lib/isc/netmgr/http.c b/lib/isc/netmgr/http.c
>> index 8f6cf21..063aa10 100644
>> --- a/lib/isc/netmgr/http.c
>> +++ b/lib/isc/netmgr/http.c
>> @@ -3529,7 +3529,9 @@ isc__nm_base64url_to_base64(isc_mem_t *mem, const char 
>> *base64url,
>>              res[i] = '/';
>>              break;
>>          default:
>> -            if (base64url_validation_table[(size_t)base64url[i]]) {
>> +            if (base64url_validation_table[(unsigned char)
>> +                                   base64url[i]])
>> +            {
>>                  res[i] = base64url[i];
>>              } else {
>>                  isc_mem_free(mem, res);
>> @@ -3587,7 +3589,7 @@ isc__nm_base64_to_base64url(isc_mem_t *mem, const char 
>> *base64,
>>               * the rest of the characters.
>>               */
>>              if (base64[i] != '-' && base64[i] != '_' &&
>> -                base64url_validation_table[(size_t)base64[i]])
>> +                base64url_validation_table[(unsigned char)base64[i]])
>>              {
>>                  res[i] = base64[i];
>>              } else {
>> diff --git a/tests/isc/doh_test.c b/tests/isc/doh_test.c
>> index fa5c9cb..c0637d9 100644
>> --- a/tests/isc/doh_test.c
>> +++ b/tests/isc/doh_test.c
>> @@ -1484,6 +1484,19 @@ ISC_RUN_TEST_IMPL(doh_base64url_to_base64) {
>>          assert_null(res);
>>          assert_true(res_len == 0);
>>      }
>> +    /*
>> +     * invalid: a byte with the high bit set must be rejected and must
>> +     * not be used as a (sign-extended) index into the validation table.
>> +     */
>> +    {
>> +        char test[] = { 'P', 'D', 'w', (char)0x80, '\0' };
>> +        res_len = 0;
>> +
>> +        res = isc__nm_base64url_to_base64(isc_g_mctx, test,
>> +                          strlen(test), &res_len);
>> +        assert_null(res);
>> +        assert_true(res_len == 0);
>> +    }
>>  }
>>    ISC_RUN_TEST_IMPL(doh_base64_to_base64url) {
> 
> Note that it's not possible to reach this code with a DoH dns parameter value 
> containing any character greater than 0x7F at present, as the parser used to 
> extract the value before calling this code is extremely strict and will 
> reject the query.  There is no other use of this code as of this writing.
> 
> While the signed issue here should be fixed in my personal opinion, this 
> issue is purely theoretical at this point.
> 
> -Doug
> --
> Visit https://lists.isc.org/mailman/listinfo/bind-users to unsubscribe from 
> this list.

-- 
Visit https://lists.isc.org/mailman/listinfo/bind-users to unsubscribe from 
this list.

Reply via email to