Re: readelf [PATCH]

[Sergei Pachkov]

Hi

Nick Clifton wrote:
> Hi Sergei,
>
> > I have found a little bug in "readelf" this weekend.  Sometimes
> > variable called "process_headers" may have NULL value
> >   
>
> err you mean 'section_headers'
>
> > and reading its members would fail.  For avoid this problem I have
> > wrote little patch.  In the patch I have add few checks of variable
> > process_headers like this: 
> > if (process_headers != NULL)
> > or
> > if (process_headers == NULL)
> >   
>
> This should not be necessary.  The functions that initialise
> section_headers (either get_32bit_section_headers or
> get_64bit_section_headers) should always return an error code if they
> failed, and this should always be checked.  So the correct fix for the
> bug is to find out where get_{32|64}bit_section_headers() is not
> returning an error code, or where their return values are being
> ignored and to fix these places.
> How did you cause readelf to attempt to use section_headers when it
> was not initialised ?
>From my frends I get elf32 binary and I try to see his info by readelf.
In binary file the parametr "elf_header.e_shoff" has mistake value and
with options -a or -A, -g readelf has failed.
I'm sorry, the test binary is very big and I don't send it to your.

call history:

main
 |
+-process_file
   |   
  +-get_file_header
  +-process_file_header
      + I have mistake value for elf_header.e_shoff which bigger than
filesize
  +-process_section_headers
     |
    +-get_32bit_section_headers
    +  |
    + +-get_data
    + +  +return NULL after invalid read from invalid space, past
current end of file
    + +return 0; before initialize section_headers;
    +return 0;

  +-process_program_headers
  +-      ........
  +-     .........
  +- process_gnu_liblist ( segfault )
> >  {
> >    PTR mvar;
> > -
> > +  int err;
> > +  int c;
> > +  
> >    if (size == 0)
> >      return NULL;
> >  
> > -  if (fseek (file, offset, SEEK_SET))
> > +  err = fseek (file, offset, SEEK_SET);
> > +  Ó = getc(file);
> > +
> > +  if ((Ó == EOF) || (err != 0))
> >   
>
> Note - this appears to be a fix/hack for a completely unrelated
> problem and should not have been included in this patch.
Hmm, sometimes offset has value more bigger than file size and this
fix/hack test seek error and report to user about it.

Sergei Pachkov