Repository navigation
Conversation
49e1b34 to
ba220ec
Compare
b48131d to
165c6aa
Compare
Some stripped Thumb binaries have an even entrypoint, which causes us to analyze them as ARM and have a hard time finding any functions at all. .ARM.attributes, if present, can occasionally help us avoid this case by providing yet another clue that the binary is intended to be run as Thumb from the get-go.
165c6aa to
bc56277
Compare
ba220ec to
afe5f96
Compare
plafosse
left a comment
There was a problem hiding this comment.
Approving but I'd like to have a firm answer on what we should do with that one length check.
| } | ||
| length -= sizeof(length); | ||
|
|
||
| auto subsection = section.Slice(section.GetOffset(), length); |
There was a problem hiding this comment.
We probably need a length check here? But maybe not as we're already in a try/catch.
There was a problem hiding this comment.
I thought about this a bit and I think I would keep it as-is. If we have a bad length here, that reaches out beyond the extent of the section, then the call to slice will clamp it to the section's bounds. This is obviously only the case for invalid binaries.
I think the question here is what we want to do with those. I would argue that we should try our best to parse them anyway, i.e. be a little lenient with their sections, since we mostly are not going to be reading most of them anyway–we are only here for one attribute. I think it would be kind of annoying if Binary Ninja just bailed out of it saw something it didn't like (but a human could typically be willing to overlook): we are not a correctness validator. There are some cases where we genuinely cannot proceed because the corruption is making it impossible for us to do our job, but in this case, we just ignore the bad data and keep going. At least as far as I can tell.
Some stripped Thumb binaries have an even entrypoint, which causes us to analyze them as ARM and have a hard time finding any functions at all. .ARM.attributes, if present, can occasionally help us avoid this case by providing yet another clue that the binary is intended to be run as Thumb from the get-go.