#225 UPX sets SizeOfHeaders field of the PE image improperly

[Marcin Wiązowski]

PE images, generated by UPX, get invalid SizeOfHeaders value in their headers.

PE file format specification says about PEHeader.OptionalHeader.SizeOfHeaders field:
"The combined size of an MS-DOS stub, PE header, and section headers rounded up to a multiple of FileAlignment".

So this is a physical size in file, not a virtual size after loading image to the memory (because field value is aligned to FileAlignment, not the SectionAlignment). But UPX uses the virtual size instead.

Most often UPX creates PE images, where the first section (UPX0) has the physical offset in file 0x200 or 0x400, and relative virtual address (RVA) is set as in the original file - which is most often 0x1000 or 0x10000. For example, for the upx.exe 3.91w itself:

Header size   00001000

Object table:
#   Name      VirtSize    RVA     PhysSize  Phys off  Flags
--  --------  --------  --------  --------  --------  --------
01  UPX0      00150000  00001000  00000000  00000200  E0000080 [ERUW]
02  UPX1      0004A000  00151000  0004A000  00000200  E0000040 [EIRW]
03  .rsrc     00001000  0019B000  00000600  0004A200  C0000040 [IRW]

As we can see, physical offset of the first section is 0x200 and RVA is 0x1000. So, in this case, header size should be set to 0x200 - but UPX sets it improperly to 0x1000.

As a reference, see Notepad.exe from Windows - it has physical offset of the first section 0x400 and header size also set to 0x400.

UPX behavior may confuse some tools, that work with PE files and rely on this value. Let's assume, that some tool tries to enlarge physical size of the header by inserting additional 0x200 bytes there. In our example with UPX, "Header size" and "Phys off" will be increased by 0x200 like this:

Header size   00001200

Object table:
#   Name      VirtSize    RVA     PhysSize  Phys off  Flags
--  --------  --------  --------  --------  --------  --------
01  UPX0      00150000  00001000  00000000  00000400  E0000080 [ERUW]
02  UPX1      0004A000  00151000  0004A000  00000400  E0000040 [EIRW]
03  .rsrc     00001000  0019B000  00000600  0004A400  C0000040 [IRW]

Although Windows can load images with header size set to 0x1000 and RVA of the first section 0x1000, it refuses loading images with header size set to 0x1200 and RVA of the first section 0x1000. So, after using a tool, image won't load anymore.

So, to solve the problem:
UPX should set PEHeader.OptionalHeader.SizeOfHeaders field equal to the physical file offset of the UPX0 section.

Regards

[László Molnár]

Unfortunately I'm not sure about this one. In theory you're right. UPX used to set SizeOfHeaders to the physical offset of the first section. But 12 years ago, for some now forgotten reason, that code was changed to use the RVA.

https://www.pysol.org:4443/hg/upx.hg/rev/0a8900c79694#l3.149
https://www.pysol.org:4443/hg/upx.hg/rev/0a8900c79694#l3.175

This code was changed because of some bug reports. At that time there were lots of strange linkers, which set funny values into the PE header, and unfortunately the PE loader in windows kernel just let them go. So in this case the documentation and the reality is not the same. I'm afraid we have to live with that.

[Marcin Wiązowski]

Thanks for your answer. I checked the links that you gave me, so I found that this change was made between UPX 1.06w and 1.07w. So I found them somewhere in the internet and compared.

So solution to the puzzle is: this change was not to correct some bug, this change was experimental. In NEWS file from upx 1.07w we can read:

Changes in 1.07 (20 Feb 2001)

  * win32/pe: experimental support for SizeOfHeaders > 0x1000

This wasn't fortunate change. There are many complaints about this in the internet, and programmers use workarounds to overcome problem with incorrect header size value:

ftp://78.46.141.148/dos/extender/hx/DOC/PESTUB.TXT
"OptionalHeader.SizeOfHeader doesn't tell the truth in any case (UPX packed binaries for example)"

http://www.wasm.ru/forum/viewtopic.php?id=45399
Whole forum thread about this

http://www.jiniya.net/wp/archives/4250
Google Translate'd: "In other words, SizeOfHeaders field trying to prove something to upx compressed file code can be dangerous"

http://pasotech.altervista.org/delphi/articolo75.htm
Comment in program code, Google Translate'd: "added to verify that the first Section coincides with the end headers [...] and this has to do with the various packers (Such as UPX, etc ...)"

http://bbs.csdn.net/topics/350021729
In program code: "if PSections[SectionLoop].PointerToRawData < HeaderSize then HeaderSize := PSections[SectionLoop].PointerToRawData;"

So this experimental change should be recognized as a mistake... Please consider reverting to the pre-1.07 state.

Regards

[Markus F.X.J. Oberhumer]

We really should NOT change the current behaviour because of compatiblity reasons.

But then, if we clearly agree on the correct header values, I do not object adding a new commandline option to enable that feature on demand.

[László Molnár]

Unfortunately reverting that change is not the correct solution since the change was added to fix a problem. The main issue here is that we no longer have the test files nor the test environments (from win95..win8) to test any changes like this. Introducing an option would be little help here, since we can not enable it by default, which means possibly few people would use it.

So I think we should not touch the x86/win32 part of code. But this change is fine for the recently introduced win64 part.