Serenity: LibELF: Spinning when trying to create a loader from a bogus ELF

Created on 28 Nov 2020  路  4Comments  路  Source: SerenityOS/serenity

Not sure what to put for the title. It doesn't spin when trying to execute it in Serenity and haven't tried it anywhere else. Just going off what FuzzELF does.

From OSS-Fuzz. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=27921&sort=-id&q=proj%3Dserenity&can=2

File: https://oss-fuzz.com/download?testcase_id=4650625660616704

Most helpful comment

sorry for off-topic.

I think the serenity devs would be much better off spending their time on adding features and fixing bugs that real users might encounter than working on addressing stuff like this.

One of the great things about this project is, there is no such thing as a "the serenity devs", IMHO. Everyone can work on whatever they would like, everyone can contribute. If @Lubrsi feels like hardening image decoders, that's perfectly fine. If I feel like fixing weird edge cases in LibJS, and I do, that's perfectly fine as well - even though they were found by fuzzers and are unlikely to affect the average website's code. :wink: And if you feel like doing "real-life" feature & bug fix work, go for it!

Bottom line is, everyone here is more or less independently working on stuff they're interested in, and the mere existence of a dozen image decoder fuzzing bugs doesn't mean anyone will fix those. Similarly, the existence of (far more than) a dozen feature request and bug reports of yours doesn't mean anyone will work on those either. It all depends on each individual contributor's work - as you know, there is no plan!


Also these issues did already exist on bugs.chromium.org anyway, and I guess moving/copying them to GitHub for increased visibility is fine.

All 4 comments

I know this is not my project and you're only trying to help but I have to question the usefulness of all these fuzzing 'bugs' which are only triggered by doing weird stuff that normal users would never do like load bogus ELF files.

To me, fuzzing is only really worthwhile on a finished, working bit of software that does what is needed of it already and so the devs can seriously justify working on fixing obscure corner case issues and security flaws.

I think the serenity devs would be much better off spending their time on adding features and fixing bugs that real users might encounter than working on addressing stuff like this. Security doesn't really matter under serenity yet IMO because its not fully featured enough to be used as anyones full time OS yet.

Not to say security isn't important. It should be planned for but I don't think it should be a priority until Serenity becomes usable as a daily driver.

Just my 2p.

sorry for off-topic.

I think the serenity devs would be much better off spending their time on adding features and fixing bugs that real users might encounter than working on addressing stuff like this.

One of the great things about this project is, there is no such thing as a "the serenity devs", IMHO. Everyone can work on whatever they would like, everyone can contribute. If @Lubrsi feels like hardening image decoders, that's perfectly fine. If I feel like fixing weird edge cases in LibJS, and I do, that's perfectly fine as well - even though they were found by fuzzers and are unlikely to affect the average website's code. :wink: And if you feel like doing "real-life" feature & bug fix work, go for it!

Bottom line is, everyone here is more or less independently working on stuff they're interested in, and the mere existence of a dozen image decoder fuzzing bugs doesn't mean anyone will fix those. Similarly, the existence of (far more than) a dozen feature request and bug reports of yours doesn't mean anyone will work on those either. It all depends on each individual contributor's work - as you know, there is no plan!


Also these issues did already exist on bugs.chromium.org anyway, and I guess moving/copying them to GitHub for increased visibility is fine.

I know this is not my project and you're only trying to help but I have to question the usefulness of all these fuzzing 'bugs' which are only triggered by doing weird stuff that normal users would never do like load bogus ELF files.

To me, fuzzing is only really worthwhile on a finished, working bit of software that does what is needed of it already and so the devs can seriously justify working on fixing obscure corner case issues and security flaws.

I think the serenity devs would be much better off spending their time on adding features and fixing bugs that real users might encounter than working on addressing stuff like this. Security doesn't really matter under serenity yet IMO because its not fully featured enough to be used as anyones full time OS yet.

Not to say security isn't important. It should be planned for but I don't think it should be a priority until Serenity becomes usable as a daily driver.

Just my 2p.

strongly disagree

ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 09 00 4c 46 01 ff 3b
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              DYN (Shared object file)
  Machine:                           Intel 80386
  Version:                           0x1
  Entry point address:               0x3b00
  Start of program headers:          54 (bytes into file)
  Start of section headers:          48 (bytes into file)
  Flags:                             0x0
  Size of this header:               52 (bytes)
  Size of program headers:           0 (bytes)
  Number of program headers:         0
  Size of section headers:           40 (bytes)
  Number of section headers:         8197
  Section header string table index: 0
readelf: Warning: possibly corrupt ELF header - it has a non-zero program header offset, but no program headers
readelf: Warning: There are 8197 section headers, starting at offset 0x30:
section 0: sh_link value of 4294967040 is larger than the number of sections
Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0] <no-name>         STRTAB          00003600 00008c 000000 ffffffff   o 4294967040 4294967295 4294967295
readelf: Warning: section 1: sh_link value of 4294967295 is larger than the number of sections
  [ 1] <no-name>         LOUSER+7fffffff ffffffff ffffffff ffffffff ffffffff WAXxMSILOGTCxxxxxxxxop 4294967295 4294967295 4294967295
readelf: Warning:   [ 2] <no-name>         LOUSER+7fffffff ffffffff ffffffff ffffffff ffffffff WAXxMSILOGTCxxxxxxxxop 4294967295 4294967295 4294967295
section 2: sh_link value of 4294967295 is larger than the number of sections
....

Looks like there's a few things wrong with this file:

  • Type is ET_DYN, but

    • Program header size is 0

    • Zero program headers

  • Section header start overlaps with program header start. Most linkers I would expect to put the section headers after the program headers. After everything, actually. They normally live at the end of the file.
  • Section header table size is 40 bytes, but there's allegedly 8197 of them.
  • Actual sections have a sh_link value that is supposed to... do things? I don't think Serenity even reads this value. But they are like all 0xFFFFFFFFF ... which is clearly bogus. > number of section headers.

I don't think that validate_elf_header or valdiate_program_headers checks for like, any of these things :)

Easy place to add these kinds of checks in LibElf/Validation.cpp though.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Arthur-Kamau picture Arthur-Kamau  路  7Comments

nvella picture nvella  路  7Comments

awesomekling picture awesomekling  路  6Comments

awesomekling picture awesomekling  路  3Comments

linusg picture linusg  路  3Comments