This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readelf/obj] - Fix the possible crash when dumping group sections.
ClosedPublic

Authored by grimar on Nov 23 2020, 12:56 AM.

Details

Summary

It is possible to trigger a crash/misbehavior when the st_name field of
the signature symbol goes past the end of the string table.

This patch fixes it.

Diff Detail

Event Timeline

grimar created this revision.Nov 23 2020, 12:56 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Nov 23 2020, 1:16 AM
llvm/test/tools/llvm-readobj/ELF/groups.test
390

I'm confused. Why do we need to do this, rather than just specifying an arbitrarily large StName value greater than the string table size?

llvm/tools/llvm-readobj/ELFDumper.cpp
3613

Perhaps this makes a bit more sense, since st_name is a value/pointer, rather than a range, so it doesn't "go" anywhere.

grimar added inline comments.Nov 23 2020, 1:25 AM
llvm/test/tools/llvm-readobj/ELF/groups.test
390

I am testing 2 cases here:

  1. (OK) When the StName == [size of string table - 1]
  2. (FAIL) When StName == size of string table

I think we should know its size for this test and test values on border.
It can be done easier when the content of the string table is known.

Also in the test below I am changing the content to remove the zero terminator, keeping st_name values the same.
This proves that we can never read past the end of the string table and that way we can't have a situation of reading
past the EOF.

jhenderson added inline comments.Nov 23 2020, 1:29 AM
llvm/test/tools/llvm-readobj/ELF/groups.test
390

Okay, perhaps worth adding to the commetn that setting the string table contents explicitly makes it easier to show the boundary conditions, or something like effect.

grimar updated this revision to Diff 306999.Nov 23 2020, 2:02 AM
grimar marked an inline comment as done.
grimar edited the summary of this revision. (Show Details)
  • Addressed review comments.
jhenderson accepted this revision.Nov 23 2020, 2:03 AM

LGTM, with nit.

llvm/test/tools/llvm-readobj/ELF/groups.test
391
This revision is now accepted and ready to land.Nov 23 2020, 2:03 AM