[WIP] resolv/res_send.c (__libc_res_nsend): Correctly sanity check buffer size.

  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?
  

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?"