Re: [B.A.T.M.A.N.] [PATCH] alfred: Tighten size check on received packet

[Jan-Philipp Litza <janphilipp@litza.de>]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

> On Monday 19 January 2015 21:59:32 Jan-Philipp Litza wrote:
>> When first checking if a received packet is truncated, the size of the
>> alfred_tlv structure is ignored, thus allowing packets that are
>> truncated by 4 bytes or less to pass the check unnoticed.
>>
>> Even the check itself might access memory after the packet if its size
>> was only 2 bytes or less.
> [...]
>>  	/* drop truncated packets */
>> -	if (length < ((int)ntohs(packet->length)))
>> +	if (length < (int)sizeof(*packet) ||
>> +	    length < (int)(ntohs(packet->length) + sizeof(*packet)))
>>  		return -1;
>>
>>  	/* drop incompatible packet */
> 
> Thanks for the patch. It is basically correct but maybe you can modify it
> slightly to make it also catch very small packets.

It already does that in the first line of the if statement. Your
modification only checks this earlier, but because no data from the
packet is accessed before the length check, it should not matter when we
do this. Or am I missing the point?

> 
> diff --git a/recv.c b/recv.c
> index 90db0b3..288f577 100644
> --- a/recv.c
> +++ b/recv.c
> @@ -391,7 +391,12 @@ int recv_alfred_packet(struct globals *globals, struct interface *interface)
>  		return -1;
>  	}
>  
> +	/* drop packets smaller than tlv */
> +	if (length < (int)sizeof(*packet))
> +		return -1;
> +
>  	packet = (struct alfred_tlv *)buf;
> +	length -= sizeof(*packet);
>  
>  	/* drop packets not sent over link-local ipv6 */
>  	if (!is_ipv6_eui64(&source.sin6_addr))
> 
> Kind regards,
> 	Sven
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]