[PATCH] hostap_plx: fix CIS verification

Jouni Malinen jkmaline
Tue Oct 24 19:31:19 PDT 2006


On Tue, Oct 24, 2006 at 10:12:24PM -0400, Pavel Roskin wrote:

> Coverity has no means to interpret CIS.  However, it may understand
> kmalloc, which allocates CIS_MAX_LEN for the CIS copy.
> 
> The value of cis[pos + 1] has no bearing on the validity of the access
> to cis[pos + 5] from the point of view of a checking tool.

Indeed.

> pos is already checked in the beginning of the loop to be small enough,
> but the check is not strong enough.  The next tuple starts at (pos +
> cis[pos + 1] + 2), and we want that to be at most CIS_MAX_LEN.
> 
> That's something Coverity could have found.

Agreed and I went through the report at some point and I think I found
it to be valid..

> So, the right fix would be:
> -		if (pos + cis[pos + 1] >= CIS_MAX_LEN)
> +		if (pos + 2 + cis[pos + 1] > CIS_MAX_LEN)
>  			goto cis_error;

> I'm rewriting this with "<" because it's easier to read.  Next tuple at
> exactly CIS_MEX_LEN is fine - we just stop precessing at that point.
> 
> I'm going to combine this with my previous fix and resend.

Great, thanks!

-- 
Jouni Malinen                                            PGP id EFC895FA




More information about the Hostap mailing list