Bug 28834 - incorrect detection of "DWARF error: section .debug_str is larger than its filesize"
Summary: incorrect detection of "DWARF error: section .debug_str is larger than its fi...
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: binutils (show other bugs)
Version: 2.37
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-01-28 21:23 UTC by Joel Hock
Modified: 2022-11-11 07:36 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joel Hock 2022-01-28 21:23:05 UTC
Commit 647cebce12a can cause valid sections to be considered corrupt if their uncompressed size is greater than the size of the entire binary, resulting in warnings like:

addr2line: DWARF error: section .debug_str is larger than its filesize! (0xb60c570 vs 0x9309705)

One example binary we have is 150MB total; however, the .debug_str section is 190MB uncompressed (and only 27MB compressed).  Iā€™m not sure what the best fix is to keep the protection of intended by the commit without causing these false positives.
Comment 1 Nick Clifton 2022-02-01 11:17:28 UTC
Hi Joel,

  Hmmm, compressed strings would indeed cause this of problem.

  Could you upload the test binary that you are using ?

  Compressed sections include a field giving their uncompressed size, so it may be possible to make use of that.  But a malicious actor could create a corrupt .zdebug_str section with a ridiculously large "real section size" field and then try to trick the application into allocating a huge amount of memory....

  Maybe a simpler solution would be change the heuristic that produces the error message so that it only complains if the .debug_str section is more than 10x the size of the file ?  Just how efficient can (zlib based) text compression get these days ?

Cheers
  Nick
Comment 2 Joel Hock 2022-02-01 20:27:28 UTC
I can't upload the binary, but the uncompressed size (b6e3fb5 in the output below) is correct.  This output is from a different, but similar, binary than what was generating the original error message I posted:

$ readelf -t mybinary
  [Nr] Name
       Type              Address          Offset            Link
       Size              EntSize          Info              Align
       Flags
...
  [38] .debug_str
       PROGBITS         0000000000000000  0000000007259234  0
       0000000001a18720 0000000000000001  0                 1
       [0000000000000830]: MERGE, STRINGS, COMPRESSED
       ZLIB, 000000000b6e3fb5, 1

A 10x heuristic would have worked in this case, fwiw.

Joel
Comment 3 cvs-commit@gcc.gnu.org 2022-02-02 17:07:08 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=41ba8b76ccc5bb01507beb3b49df039264bcf34a

commit 41ba8b76ccc5bb01507beb3b49df039264bcf34a
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Feb 2 17:06:22 2022 +0000

    Stop the BFD library complaining about compressed dwarf debug string sections being too big.
    
            PR 28834
            * dwarf2.c (read_section): Change the heuristic that checks for
            overlarge dwarf debug info sections.
Comment 4 cvs-commit@gcc.gnu.org 2022-02-02 17:08:12 UTC
The binutils-2_38-branch branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=203c99c9b9a9c58cce472a1bd4315e3f79b440b7

commit 203c99c9b9a9c58cce472a1bd4315e3f79b440b7
Author: Nick Clifton <nickc@redhat.com>
Date:   Wed Feb 2 17:07:31 2022 +0000

    Stop the BFD library from complaining that dwarf debug string sections are too big.
    
            PR 28834
            * dwarf2.c (read_section): Change the heuristic that checks for
            overlarge dwarf debug info sections.
Comment 5 Nick Clifton 2022-02-02 17:09:33 UTC
Hi Joel,

  Well the 10x fix is easy to implement, and should be safe, so I have
  gone ahead and applied it.

Cheers
  Nick
Comment 6 cvs-commit@gcc.gnu.org 2022-11-11 07:36:23 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=f7502dfe3f99d09fba2fc49f806ccc6b0a18c06d

commit f7502dfe3f99d09fba2fc49f806ccc6b0a18c06d
Author: Alan Modra <amodra@gmail.com>
Date:   Fri Nov 11 13:43:42 2022 +1030

    PR28834, PR26946 sanity checking section size
    
    This patch provides a new function to sanity check section sizes.
    It's mostly extracted from what we had in bfd_get_full_section_contents
    but also handles compressed debug sections.
    Improvements are:
    - section file offset is taken into account,
    - added checks that a compressed section can be read from file.
    
    The function is then used when handling multiple .debug_* sections
    that need to be read into a single buffer, to sanity check sizes
    before allocating the buffer.
    
            PR 26946, PR 28834
            * Makefile.am (LIBBFD_H_FILES): Add section.c.
            * compress.c (bfd_get_full_section_contents): Move section size
            sanity checks..
            * section.c (_bfd_section_size_insane): ..to here.  New function.
            * dwarf2.c (read_section): Use _bfd_section_size_insane.
            (_bfd_dwarf2_slurp_debug_info): Likewise.
            * Makefile.in: Regenerate.
            * libbfd.h: Regenerate.