Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix multiple occurrences of bad pack/unpack specifiers #3484

Merged
merged 4 commits into from Jul 16, 2014

Conversation

hdm
Copy link
Contributor

@hdm hdm commented Jun 30, 2014

These prevent Metasploit from working properly on big-endian architectures and a number of these have crept in over the years.

HD Moore added 3 commits June 30, 2014 02:46
Ruby treats endianess in pack operators in the opposite way
of python. For example, using pack('<I') actually ignores the
endianess specifier. These need to be 'I<' or better yet, 'V'.
The endian specify must occur after the pack specifier and
multiple instances in meterpreter and exe generation were
broken in thier usage.

The summary:

Instead of I/L or I< use V
Instead of I/L or I> use N
For Q, you need to always use Q< (LE) or Q> (BE)
For c/s/l/i and other lowercase variants, you probably dont
need or want a *signed* value, so stick with vV nN and cC.
Note that there are some cases of host-endian left, these
are intentional because they operate on host-local memory
or services.

When in doubt, please use:

```
ri pack
```
@hdm
Copy link
Contributor Author

hdm commented Jun 30, 2014

Interesting test failure:

       -/ELF 32-bit LSB executable, MIPS/
       +/dev/stdin: ELF 32-bit LSB  executable, MIPS, MIPS-I version 1 (SYSV), statically linked, corrupted section header size

The extra space between LSB/MSB and "executable" is what is causing this test failure. The "corrupted section header size" output in the output of master as well. I will double check that master generates the same binaries and then fix the spec instead.

@hdm
Copy link
Contributor Author

hdm commented Jun 30, 2014

Master does indeed fail with the same spec error. It looks like a change to file(1) output. Correcting the spec but also spot checking the generated binaries in case the corruption section header size problem is valid.

@hdm
Copy link
Contributor Author

hdm commented Jun 30, 2014

More proof that master was going to fail this spec and there are no changes to the resulting payloads:

149eedf8fae2a26ffb915ab71e6a0f5528637f0f  /tmp/be.bin
149eedf8fae2a26ffb915ab71e6a0f5528637f0f  /tmp/be_master.bin
717f19b8e7cdc33e18ec4601f85a46491bf1985b  /tmp/le.bin
717f19b8e7cdc33e18ec4601f85a46491bf1985b  /tmp/le_master.bin

@Meatballs1
Copy link
Contributor

Maybe a note in hacking/contributing/wiki. Wasnt really aware of the issue

@jlee-r7
Copy link
Contributor

jlee-r7 commented Jul 3, 2014

@hmoore-r7 The corrupt header size warning is an artifact of how we build ELF files. The binaries should still work and produce sessions.

@jlee-r7
Copy link
Contributor

jlee-r7 commented Jul 3, 2014

Also, I really hate those file(1) specs, but we don't have any other reasonable means of ensuring that the generated output isn't completely insane.

@wvu wvu self-assigned this Jul 8, 2014
@wvu wvu merged commit 4ff211e into rapid7:master Jul 16, 2014
wvu added a commit that referenced this pull request Jul 16, 2014
@Meatballs1
Copy link
Contributor

Have noticed Railgun uses Native Q:

#3550

@Meatballs1
Copy link
Contributor

@hmoore-r7 do you have the regex for this to put into msftidy?

@hdm hdm deleted the bug/fix-host-endianess-in-pack branch August 26, 2014 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants