COFF: bound a section's raw data by the size of the file - #806
Conversation
A COFF section header says where a section's raw data starts and how much of it there is. The backend used both fields as they came, so an object can declare more raw data than the file holds and cle builds a section and a segment for memory nothing backs. A 108-byte object stating SizeOfRawData 0x4000000 gets a segment claiming 0x4000000 bytes, and max_addr reaches 0x440003b where the image ends at 0x40006b. Both fields are 32 bits wide. Map only what the file holds, and skip a section the file does not reach at all. The PE backend already bounds the same two fields this way; the COFF path never got the check. PointerToRawData 0 says the section has no raw data in the file and gives its size in memory instead, which the file does not bound, so that case is left as it was.
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS Validation record for head A corpus sweep holds the primary checkouts and the shared virtualenv on this Imports resolved to the branch, not to the workspace install: cle suite -- Merge-base lint and type comparison -- Pre-commit -- Test inputs -- Before and after, on
The image is 108 bytes on both revisions in both cases. All eight On the head of #804, which builds the image rather than mapping the file, the On master session: sharpen |
|
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS The loaded image, segments and sections for the reproducer in the description: printing
|
|
Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/cle_806 |
Coff._add_relocs took the patch address as section.PointerToRawData plus reloc.VirtualAddress and registered a relocation there without checking it. Neither bound was tested, and the two fail differently. A field past the end of the file crashes. The backend maps the object as one backer covering the file, so CoffRelocationDIR32.value asks Clemory for four bytes at an address nothing maps, and cle.Loader(..., perform_relocations=True) raises KeyError out of Clemory.load. A field merely past the end of its own section does not crash, and that is the worse half. Every offset in the file is mapped, so the store lands wherever the arithmetic points -- another section's raw data, the relocation table, the symbol table -- and the load returns normally with those bytes rewritten. Check both bounds where the relocation is registered rather than in relocate(). A relocation that cannot be applied should not reach self.relocs at all: it is handed to the symbol resolver, it can produce an extern symbol for a field that will never be written, and it is visible to every consumer that iterates an object's relocations. It is also where the PE backend drops a section whose raw data the file does not hold. The field's width comes from struct.calcsize on the relocation class's PACK_FORMAT -- four bytes normally, eight for ADDR64, two for SECTION -- so a four-byte field starting on the last byte of a section is out of bounds, which a bound on the start offset alone would miss. PACK_FORMAT is declared on CoffRelocation rather than on Relocation, so RELOC_CLASSES is annotated with the class it actually holds. This leaves the section mapping loop alone. Bounding a section's raw data by the size of the file is #806; the two compose, because _add_relocs walks self._coff.sections itself and would still register the relocations of a section that loop has skipped. Two details keep this bound correct against the other open COFF branches, and change nothing on this one. The section comes out of self._coff.sections by index rather than off the loop variable. Both name the same object here, by the definition of enumerate. #764 rewrites this loop to walk indices and drops the variable, and the two branches merge with no textual conflict, so with both applied and the loop variable read _add_relocs raises NameError on the first relocation of a supported type. Of the five COFF objects angr/binaries tracks that this backend loads, four carry such a relocation and stop loading; the fifth has none. #804 is stacked on #764 and carries the same rewrite. The file-size half of the bound is taken against self._image_vmem, the bytes the backend maps, rather than against self._data. Here the two are the same object: _image_vmem is assigned from _data in __init__, never rebound, and cle defines no subclass of Coff. #804 places a section whose file offset does not satisfy its alignment past the end of the file and extends the image to cover it, so a relocation into a moved section is past len(self._data) and inside the image, and bounding on the file would skip it. With both applied and the file used, x86/fauxware.obj keeps 177 of its 225 relocations and x86_64/fauxware.obj 66 of 126, and the test below asserting 225 fails.
2 participants
THIS MESSAGE WAS GENERATED BY AN AUTOMATED PROCESS
Problem
A COFF section header says where a section's raw data starts and how much of it
there is, and cle checks neither field against the file.
Take
tests/x86/coff_reloc_dir32.obj, 108 bytes with one section, and set thatsection's
SizeOfRawDatato0x4000000. On master2f7657fdcle believes it:A 108-byte file gets a segment and a section claiming 64 MiB of memory that
nothing backs, and
max_addrlands 64 MiB past the end of the image. SetPointerToRawDatato0x4000000instead and the section is placed 64 MiB outwhole.
SizeOfRawDatais 32 bits wide, so a header can claim close to 4 GiBthis way.
None of that allocates on master. The COFF image is the file itself, so the
header buys a wrong extent rather than memory: the first
0x30bytes are realand read fine, and everything the section claims past them is unbacked, so
ld.memory.load(0x40006c, 4)raisesKeyError.It stops being only an extent once the backend materialises what the header
states, which is what #804 does. #804 is unmerged, so this is not something cle
does today. On its head, with the section additionally marked
IMAGE_SCN_ALIGN_512BYTESso that its file offset0x3cfails its own statedalignment and the realignment branch runs, that same 108-byte object produces a
67,109,376 byte image and takes the process from 70 MB to 262 MB RSS over three
runs. Bounding the field is cheaper before that lands than after.
Root cause
Coff.__init__copies both fields straight out of the section table:Nothing between the header and the
SegmentcomparesPointerToRawData + SizeOfRawDataagainstlen(self._data). The PE backend does compare them, in_get_memory_mapped_imageincle/backends/pe/pe.py: it cuts a section the fileends inside, and skips one that starts past the end. The COFF path never got
that check.
Fix
Map what the file holds, and skip a section the file does not reach at all --
the same two outcomes PE has. The two objects above:
tests/x86/fauxware.objis untouched either way: image 16,676 bytes,max_addr0x4031c0.Both
filesizeandmemsizeare clamped. A COFF section header does carry aVirtualSize, but it is zero in every section of all five COFF objectsangr/binariestracks that this backend can load, soSizeOfRawDatais theonly size on offer and an unclamped
memsizeis memory with nothing behind it.The header's own value is still readable as
section._coff_sec.SizeOfRawData.Deliberately not done:
PointerToRawData0 says the section has no raw data inthe file at all and gives its size in memory instead, which the file does not
bound. That case is left alone, so a section stating it keeps whatever
SizeOfRawDataits header asks for. Whether anything is allocated for one is#764's: it gives space of its own only to a section also marked
IMAGE_SCN_CNT_UNINITIALIZED_DATA, and caps that space atMAX_IMAGE_SIZE.Also left alone: a section stating
SizeOfRawData0 at aPointerToRawDatapast the end of the file is still kept, at an address the image does not cover,
exactly as on master.
The bound is a three-argument function rather than an inline expression so that
the test below can reach it.
Merge order
Seven open pull requests touch
cle/backends/coff.py: #724, #761, #764, #775,#799, #804 and #807. Against this branch #724, #764 and #804 conflict in
coff.py, and #724, #761, #764, #775 and #804 conflict intests/test_coff.py;#799 and #807 are clean. Whichever lands second resolves them, and the
resolution in
coff.pyis to keep the bounded size. On #804 that is one line:Applied that way, #804's 67,109,376 byte image becomes 560 bytes and RSS does
not move. That line alone is not the whole resolution: without the skip beside
it, a section the file does not reach still gets a zero-length segment at an
address outside the image, and
max_addrlands below that segment's ownvaddr. Carry both.#775 caps
max_addrat the image's own length, so on a tree carrying it themax_addrline above already reads0x40006b; thefilesizeandmemsizelines hold either way.
Testing
tests/test_coff.pyasserts the bound at its four boundaries -- raw data insidethe file, cut short by it, starting past its end, and the
PointerToRawData0carve-out. That is the test that fails on master, where the function does not
exist. A second asserts that a well-formed object still maps every byte its
sections declare, checked against the file's own bytes; it passes on master too,
and guards against the bound cutting into a real object.
No fixture reaches the bound through
cle.Loaderand none can be built fromreal toolchain output, because a well-formed object never overruns. Truncating a
real one does not reach the section loop either: raw data ends at or before the
symbol table in all five COFF objects
angr/binariestracks that this backendcan load, so a file cut short enough to overrun a section has also lost its
string table, which
CoffParser._parsereads first.tests/x86/fauxware.objcut to
0x2000bytes raisesstruct.errorthere.Validation: #806 (comment)
session: sharpen