[WIP] resolv/res_send.c (__libc_res_nsend): Correctly sanity check buffer size.
Commit Message
Florian,
I had this sitting around in my tree as a fix I'd been carrying around
but for one reason or another I never had a chance to push it.
Given that the answer buffer is no longer being used for space to store
two answers the "< 2 * HFIXEDSZ" possible outcome below is now overly
conservative. We need only consider the case where (a) the buffer can't
be reallocated (ansp is NULL) and (b) the size is less than HFIXEDSZ.
For the second buffer we're always going to malloc enough space.
Thoughts?
Comments
On 05/31/2016 09:19 PM, Carlos O'Donell wrote:
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 869294f..3f42313 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -359,7 +359,9 @@ __libc_res_nsend(res_state statp, const u_char *buf, int buflen,
> return (-1);
> }
>
> - if (anssiz < (buf2 == NULL ? 1 : 2) * HFIXEDSZ) {
> + /* If the buffer can't be changed, and it's less than the
> + minimal header size, then that's an error. */
> + if (anssiz < HFIXEDSZ && ansp == NULL) {
> __set_errno (EINVAL);
> return (-1);
> }
Maybe add a comment why we don't compare against the query size? The
code might be used to send DNS UPDATE requests (although this isn't
really supported), and the query might contain a large OPT RR which is
not expected to be copied into the answer.
Florian
On 06/03/2016 04:36 AM, Florian Weimer wrote:
> On 05/31/2016 09:19 PM, Carlos O'Donell wrote:
>
>> diff --git a/resolv/res_send.c b/resolv/res_send.c
>> index 869294f..3f42313 100644
>> --- a/resolv/res_send.c
>> +++ b/resolv/res_send.c
>> @@ -359,7 +359,9 @@ __libc_res_nsend(res_state statp, const u_char *buf, int buflen,
>> return (-1);
>> }
>>
>> - if (anssiz < (buf2 == NULL ? 1 : 2) * HFIXEDSZ) {
>> + /* If the buffer can't be changed, and it's less than the
>> + minimal header size, then that's an error. */
>> + if (anssiz < HFIXEDSZ && ansp == NULL) {
>> __set_errno (EINVAL);
>> return (-1);
>> }
>
> Maybe add a comment why we don't compare against the query size? The
> code might be used to send DNS UPDATE requests (although this isn't
> really supported), and the query might contain a large OPT RR which
> is not expected to be copied into the answer.
Right, this is only a "minimum size" sanity check, but you're right
one might ask "Why not check against the query size?"
@@ -359,7 +359,9 @@ __libc_res_nsend(res_state statp, const u_char *buf, int buflen,
return (-1);
}
- if (anssiz < (buf2 == NULL ? 1 : 2) * HFIXEDSZ) {
+ /* If the buffer can't be changed, and it's less than the
+ minimal header size, then that's an error. */
+ if (anssiz < HFIXEDSZ && ansp == NULL) {
__set_errno (EINVAL);
return (-1);
}