Re: [PATCH] xt_recent: Fix buffer overflow

[Patrick McHardy <kaber@trash.net>]

Tim Gardner wrote:
> Jan Engelhardt wrote:
>> On Friday 2010-02-19 18:48, Tim Gardner wrote:
>>> Consider the case when ip_pkt_list_tot==1; the first packet received is stored
>>> in e->stamps[0] and e->index is initialized to 1. The next received packet
>>> timestamp is then stored at e->stamps[1] in recent_entry_update(),
>>> a buffer overflow because the maximum e->stamps[] index is 0.
>>> @@ -173,10 +173,10 @@ recent_entry_init(struct recent_table *t, const union nf_inet_addr *addr,
>>>
>>> static void recent_entry_update(struct recent_table *t, struct recent_entry *e)
>>> {
>>> +	e->index %= ip_pkt_list_tot;
>>> 	e->stamps[e->index++] = jiffies;
>>> 	if (e->index > e->nstamps)
>>> 		e->nstamps = e->index;
>>> -	e->index %= ip_pkt_list_tot;
>>> 	list_move_tail(&e->lru_list, &t->lru_list);
>>> }
>> Let's analyze in 3-step manner:
>>
>> Claim: writes always happen to e->stamps[0]
>> Prereqs: ip_pkt_list_tot==1
>> Proof:
>>  Start with assumption that e->index's possible values at the
>>  start of the function are {0}.
> 
> This assumption is the root of the bug. e->index is initialized to 1 in
> recent_entry_init() which means that its already out of bounds when next
> recent_entry_update() is called.

This patch looks correct to me. Applied, thanks Tim.