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

MIPS PLT Offset off by one entry #4776

Closed
elbee-cyber opened this issue Nov 26, 2023 · 11 comments
Closed

MIPS PLT Offset off by one entry #4776

elbee-cyber opened this issue Nov 26, 2023 · 11 comments
Assignees
Labels
Arch: MIPS Issues with the MIPS architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround Type: Bug Issue is a non-crashing bug with repro steps
Milestone

Comments

@elbee-cyber
Copy link

For mipsel binaries there seems to be an issue with loading only the plt (the rest of the binary seems fine) where plt entries resolve by the entry adjacent.

Tested on 3.5.4526 Personal and 3.6.4665-dev

@xusheng6
Copy link
Member

@elbee-cyber could you please share a binary so that we can better reproduce it?

@xusheng6 xusheng6 added the State: Blocked (Customer) Issue is blocked on waiting for a response from a customer label Nov 27, 2023
@fuzyll fuzyll added Type: Bug Issue is a non-crashing bug with repro steps Component: Architecture Issue needs changes to an architecture plugin Arch: MIPS Issues with the MIPS architecture plugin labels Nov 27, 2023
@elbee-cyber
Copy link
Author

The following are some MIPS binaries from a device I was doing research on
mips_binaries.zip

@psifertex
Copy link
Member

Can you be more specific as to which exhibit this problem? A quick look through several and they all look fine to me unless I'm misunderstanding the issue:

Screenshot 2023-11-27 at 20 47 11 Screenshot 2023-11-27 at 20 47 40 Screenshot 2023-11-27 at 20 48 18 Screenshot 2023-11-27 at 20 48 59

@psifertex
Copy link
Member

A quick look doesn't show any sections with "plt" in the name either if that's required?

>>> import binaryninja
>>> import glob
>>> binaryninja.disable_default_log()
>>> for file in glob.glob("*"):
...   with binaryninja.load(file, update_analysis=False) as bv:
...     print(bv)
...     for section in bv.sections:
...       if "plt" in section:
...         print(f"\t{section}")
...
<BinaryView: 'cmd_cron', len 0x2cd>
<BinaryView: 'ip6tables-save', start 0x400000, len 0x1a084>
<BinaryView: 'updatednsip', start 0x400000, len 0x19924>
<BinaryView: 'net-scan', start 0x400000, len 0x13814>
<BinaryView: 'hostapd', start 0x400000, len 0x80984>
<BinaryView: 'brctl', start 0x400000, len 0x16fd4>
<BinaryView: 'ssmtp', start 0x400000, len 0x1a6b4>
<BinaryView: 'aclctl', start 0x400000, len 0x119f4>
<BinaryView: 'dhcp6c', start 0x400000, len 0x3cd04>
<BinaryView: 'ntpst', start 0x400000, len 0x13214>
<BinaryView: 'update_smb', start 0x400000, len 0x135b4>
<BinaryView: 'miniupnpd', start 0x400000, len 0x21454>
<BinaryView: 'ez-ipupdate', start 0x400000, len 0x1bd84>
<BinaryView: 'chat', start 0x400000, len 0x15a04>
<BinaryView: 'ppp-nas', start 0x400000, len 0x13c54>
<BinaryView: 'proftpd', start 0x400000, len 0xa54f4>
<BinaryView: 'ripngd', start 0x400000, len 0x53d24>
<BinaryView: 'iwspy', len 0x0>
<BinaryView: 'apply_config', len 0x56e>
<BinaryView: 'net-dump', start 0x400000, len 0x11854>
<BinaryView: 'crond', len 0x0>
<BinaryView: 'ebtables', start 0x400000, len 0x10ca4>
<BinaryView: 'radvdump', start 0x400000, len 0x13f64>
<BinaryView: 'zebra', start 0x400000, len 0x51034>
<BinaryView: 'dns-hijack', len 0x107>
<BinaryView: 'wget_netgear', len 0x1f2>
<BinaryView: 'update_user', len 0x357>
<BinaryView: 'aclhijackdns', start 0x400000, len 0x11be4>
<BinaryView: 'uhttpd', start 0x400000, len 0x20ca4>
<BinaryView: 'px5g', start 0x400000, len 0x1fa74>
<BinaryView: 'wpa_cli', start 0x400000, len 0x1d014>
<BinaryView: 'iwlist', len 0x0>
<BinaryView: 'inetd', start 0x400000, len 0x122b4>
<BinaryView: 'select_partition', len 0x2c0>
<BinaryView: 'iwgetid', len 0x0>
<BinaryView: 'iwpriv', len 0x0>
<BinaryView: 'potval', start 0x400000, len 0x13214>
<BinaryView: 'acl_update_name', start 0x400000, len 0x10c44>
<BinaryView: 'tc', start 0x400000, len 0x4cd24>
<BinaryView: 'net-wall', start 0x400000, len 0x27c24>
<BinaryView: 'dhcp6ctl', start 0x400000, len 0x13d04>
<BinaryView: 'dev-scan', start 0x400000, len 0x10ff4>
<BinaryView: 'iwconfig', start 0x400000, len 0x20954>
<BinaryView: 'usbled', start 0x400000, len 0x11014>
<BinaryView: 'ntpclient', start 0x400000, len 0x17ec4>
<BinaryView: 'utelnetd', start 0x400000, len 0x129a4>
<BinaryView: 'ip6tables', start 0x400000, len 0x199f4>
<BinaryView: 'stamac', start 0x400000, len 0x13214>
<BinaryView: 'ipp', start 0x400000, len 0x12664>
<BinaryView: 'telnetenable', start 0x400000, len 0x13e14>
<BinaryView: 'wpa_supplicant', start 0x400000, len 0x83c54>
<BinaryView: 'usb_cfg', start 0x400000, len 0x15f34>
<BinaryView: 'minidlna', start 0x400000, len 0x5b2c4>
<BinaryView: 'net-cgi', start 0x400000, len 0xdfe54>
<BinaryView: 'dhcp6s', start 0x400000, len 0x39f24>
<BinaryView: 'noip2', start 0x400000, len 0x1f5c4>
<BinaryView: 'vol_id', start 0x400000, len 0x13c14>
<BinaryView: 'ip6tables-restore', start 0x400000, len 0x1ad94>
<BinaryView: 'chroot', len 0x0>
<BinaryView: 'restart_ap_udhcpc', len 0x220>
<BinaryView: 'iptables', start 0x400000, len 0x19cd4>
<BinaryView: 'potd', start 0x400000, len 0x13214>
<BinaryView: 'detwan', start 0x400000, len 0x19594>
<BinaryView: 'net-disk', start 0x400000, len 0x12bb4>
<BinaryView: 'acld', start 0x400000, len 0x13504>
<BinaryView: 'start_br_udhcpc', len 0x314>
<BinaryView: 'ip', start 0x400000, len 0x43ca4>
<BinaryView: 'pppd', start 0x400000, len 0x91964>
<BinaryView: 'nmbd', len 0x127b44>
<BinaryView: 'hostapd_cli', start 0x400000, len 0x15db4>
<BinaryView: 'ripd', start 0x400000, len 0x16fe4>
<BinaryView: 'radvd', start 0x400000, len 0x257c4>
<BinaryView: 'lld2d', start 0x400000, len 0x2b9b4>
<BinaryView: 'dsyslog', len 0x4e2>
<BinaryView: 'smbd', len 0x34c184>
<BinaryView: 'phddns', start 0x400000, len 0x28e04>
<BinaryView: 'dnsmasq', start 0x400000, len 0x33034>
<BinaryView: 'ntgrddns', start 0x400000, len 0x20074>

@fuzyll fuzyll removed the State: Blocked (Customer) Issue is blocked on waiting for a response from a customer label Nov 28, 2023
@fuzyll
Copy link
Contributor

fuzyll commented Nov 28, 2023

This was previously fixed in #3267 and I am definitely seeing an issue on 3.6.4671-dev with the supplied files:

image

This is from nmbd, if anyone wants to follow along, and matches the previous issue exactly. Jordan's other copied data is from the GOT, which is fine.

This has also been reported by an Enterprise customer on MIPS64 binaries.

@fuzyll fuzyll added this to the Dorsai milestone Nov 28, 2023
@fuzyll fuzyll added Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround labels Nov 28, 2023
@psifertex
Copy link
Member

We've had a discussion internally about this and come to a few conclusions.

First, yup, there's still a bug with the improved recognition that we need to fix. Ironically, for this particular binary (not sure about others) though you shouldn't ever need these to be fixed since the PLT isn't used at all. Instead, the cross-references (which are presumably what matters here) are all going to be directly off the GOT entry itself.

The bigger issue is probably that when you want to find the cross-references for an import you should be navigating to the GOT entry. This means if you search, say, "memcpy" in the sidebar, you'd want to hit the second entry, not the first one. Likewise, if you use "g" to navigate to the right function, you'd want to navigate to the one in the .got section (which is shown in the drop-down). We're discussing some ways we can improve the default behavior so you're less likely to run into this in the first place, but let me know if either of those work for you!

@elbee-cyber
Copy link
Author

Oh I've not had issues following cross references. (I can just click an imported symbol as it appears in the IL pane and follow the references) It's not too impactful of a bug, I just thought I'd report it anyway. It only makes following cross references via the plt a bit wonkey which is what my and I'm sure some other people's brains default to when they want to just follow some imported symbol off rip via the plt.

@psifertex
Copy link
Member

That makes sense, just making sure it wasn't blocking anything.

@lwerdna
Copy link
Contributor

lwerdna commented Dec 12, 2023

Note dump before I forget stuff:

The enhancements that corrected the the previous issue were activated here, but the type of our RTL_Resolve() inserted at the start of the GOT was not propagating. Peter recognized this was a section semantics issue after seeing the odd placement of RTL_Resolve(), about 8 bytes prior to the GOT:

image

Since it's not in a section, it doesn't adopt the ReadOnlyDataSectionSemantics:

>>> bv.get_sections_at(0x115648)
[<section .got_recovered_115648: 0x115648-0x116bf0>]
>>> bv.get_sections_at(0x115648)[0].semantics
<SectionSemantics.ReadOnlyDataSectionSemantics: 2>

I learned it is not necessary to have sections at all. You can just have program headers (segments) instruct the loader to map the necessary bits of the file to memory, and then you can communicate the location of important parts via the program header types and their contents. For example, the location of the GOT is communicated with a program header with type PT_DYNAMIC, which then has a dynamic entry type DT_PLTGOT:

image

When the GOT is specified this way, Binja synthesizes the ".got_recovered_XXX" section. Note the address in .got_recovered_115648 disagrees with the ELF file who specifies 0x115640. So this reduces to fixing got recovery in elfview.

@lwerdna
Copy link
Contributor

lwerdna commented Dec 12, 2023

Minor correction: view-elf was calculating recovered GOT section by collecting all relocations and inferring a sort of bounding box around them. This would miss the very first pointer in the GOT, where RTL_Resolve() is placed.

The area shown in the screenshot by fuzyll now looks like:

image

Fixed in: Vector35/view-elf@5823606 and https://github.com/Vector35/binaryninja/commit/ca11e60e10d878a86a5cb6248a7d043fca580e82

@lwerdna lwerdna closed this as completed Dec 12, 2023
@nshp
Copy link
Contributor

nshp commented Dec 12, 2023

There's still a problem with typing there - those RTL_Resolve calls should just be resolved to extern calls, otherwise every external function takes 0 parameters and doesn't return.

RTL_Resolve is part of the loading process and provided by the loader, it's not actually runtime-dynamic even though it looks that way. If I remember correctly the symbol index there is the index into the dynamic symbols table, which you could use to statically resolve which extern it should call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: MIPS Issues with the MIPS architecture plugin Component: Architecture Issue needs changes to an architecture plugin Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround Type: Bug Issue is a non-crashing bug with repro steps
Projects
None yet
Development

No branches or pull requests

6 participants