[ECOS] RE: Fixes to RedBoot "load" command

[ECOS] RE: Fixes to RedBoot "load" command

[Gary Parnes]

I see a potential vulnerability in the CYG_ASSERT() that is watching for
code that overshoots the opts[] array.  It is checking the value of
num_options against a constant.  But, num_options is also resident on the
stack.  Writing beyond the bounds of the opts[] array COULD end up
corrupting the value of num_options itself (it all depends on how the
compiler arranges things on the stack), and so it could result in a "false
positive" in the CYG_ASSERT().

I starting to think that the options mechanism needs to be reworked.
Perhaps the opts[] array could be embedded in a structure that tracks the
count and the max?

--Gary Parnes

SENIOR SOFTWARE ENGINEER

Logic Product Development
411 Washington Ave. North, Suite 101
Minneapolis, MN 55401

  Main: (612) 672-9495
Direct: (612) 436-5165



> -----Original Message-----
> From: Gary Thomas [mailto:gary@mlbassoc.com]
> Sent: Friday, April 23, 2004 3:38 PM
> To: Gary Parnes
> Cc: eCos patches
> Subject: Re: Fixes to RedBoot "load" command
> 
> 
> On Fri, 2004-04-23 at 13:43, Gary Parnes wrote:
> > Two fixes concerning RedBoot's "load" command in this 
> patch.  One corrects a
> > potential stack corruption situation.  The other fixes a 
> problem when
> > specifying the port on a little endian system.
> > 
> > 
> >  <<redboot_patch.txt>> 
> 
> 
> Thanks for pointing these out.  I've committed the change to the TFTP 
> code as-is.  The change for 'load' was rather messy so I did 
> it a little
> differently.  I also went ahead and made the same change 
> everywhere that
> a variable option list was used.
> 
> -- 
> Gary Thomas <gary@mlbassoc.com>
> MLB Associates
> 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] RE: Fixes to RedBoot "load" command

[Gary Thomas]

On Fri, 2004-04-23 at 14:56, Gary Parnes wrote:
> I see a potential vulnerability in the CYG_ASSERT() that is watching for
> code that overshoots the opts[] array.  It is checking the value of
> num_options against a constant.  But, num_options is also resident on the
> stack.  Writing beyond the bounds of the opts[] array COULD end up
> corrupting the value of num_options itself (it all depends on how the
> compiler arranges things on the stack), and so it could result in a "false
> positive" in the CYG_ASSERT().

Appreciated, but how far must one go?  Stack overflows can cause all 
sorts of erratic behaviour and little is ever certain after it occurs.

The assert clearly states what the requisites are.  If someone alters
a routine which uses such a variable array, it should be clear from
reading the code what to be careful of.  If he doesn't read the code,
then the peril is on only himself.

> 
> I starting to think that the options mechanism needs to be reworked.
> Perhaps the opts[] array could be embedded in a structure that tracks the
> count and the max?

I think that it's already too heavy (it got out of hand over time) and
I have no inclination to make it more so.

Thanks for your input and the discovery of the error which has now been
repaired.

-- 
Gary Thomas <gary@mlbassoc.com>
MLB Associates


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss