[MODERATED] Re: spectrev1+

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Thu, May 31, 2018 at 08:55:12PM +0200, speck for Jiri Kosina wrote:
> On Thu, 31 May 2018, speck for Josh Poimboeuf wrote:
> 
> > > Thanks; so my understanding was that Dan's current smatch rule 
> > > contains 'is_read()' check somewhere inside it, and therefore would 
> > > basically warn currently only about patterns a-la
> > > 
> > > 	foo = bar[user_index];
> > > 
> > > Dropping that 'is_read()' seemed like what will be needed to identify 
> > > these new patterns, and he said he'll investigate it, but then I never 
> > > heard back.
> > > 
> > > So if you can ping him from a different direction, that'd be helpful. 
> > 
> > Why would you drop the is_read()?  The gadget involves both a write and
> > a read, so the is_read() wouldn't exclude the pattern, would it?
> 
> The is_read() check is there AFAIK for the user-controlled value 
> (user_index above), while this new variant requires storing the attacker 
> specified value, so such smatch rule wouldn't trigger.
> 
> But this all should better be clarified by Dan ... 

My (probably wrong) understanding is that the pattern is something like
this:

if (user_index < array_len) {
	bar[user_index] = blah;
	...
	foo = bar[user_index];
}

Do you have a different understanding?

-- 
Josh