[PATCH 0/2] xfrm: fix buffer overflows

[Thomas Egerer]

Hello Thomas,

On 06/25/2016 02:02 PM, Thomas Haller wrote:
> On Tue, 2016-05-31 at 17:29 +0200, Thomas Egerer wrote:
>> Hi *,
>>
>> we have found one definite and one potential buffer overflow
>> in libnl when adding xfrm states.
>> The definite one is triggered whenever an aead/auth (etc) key
>> is added to an xfrmnl_sa structure. The potential one is only
>> triggered if the same functions are called with alg_names
>> longer than 72/68 bytes + keysize. Then a strcpy call writes
>> beyond the appropriate data structures in struct xfrmnl_sa.
>>
>> Cheers,
>> Thomas
>>
>> Thomas Egerer (2):
>>   xfrm: fix buffer overflow when copying keys
>>   xfrm: check length of alg_name before strcpying it
>>
> 
> 
> Hi Thomas,
> 
> 
> thanks for the 2 patches. Merged both to master:
> https://github.com/thom311/libnl/commit/c009b20919562e6968969309049064d59d35010a
> 
> 
> 
> xfrmnl_sa_get_comp_params() et al. seems wrong too.
> How can somebody safely know the required size of the @key buffer?
If changing the API of this part of the code is not a problem, then
the prototype could be changed such that the key pointer (char *)
becomes a char ** which is allocated by the function and has to be
freed by the user. Another option -- adding another maxlen parameter
to indicate the size of the key buffer -- also means an API change.

> The required size for @alg_name is also not documented, so a user would
> have to read the source to see that it must be >= 64 bytes
I don't see a problem here. The kernel API is very similar and applications
use the sizeof operator to obtain the size of the name buffer. Or am I
getting you wrong?

Thomas