PE: parse thread local storage - TLS data #404
Conversation
| })?; | ||
| let mut i = 0; | ||
| // Read the callbacks until we find a null terminator | ||
| loop { |
There was a problem hiding this comment.
on a malformed binary could this cause an infinite loop?
There was a problem hiding this comment.
Yes. I think it would make sense to have a RVA validation there and return the malformed error if invalid RVAs found.
There was a problem hiding this comment.
so what i meant specifically is: is this loop guaranteed to terminate? this comes up when there are malformed binaries, and the loop only terminates in a well-formed case, but if the loop has no upper bound, it might cause the library to run forever, which is unacceptable.
There was a problem hiding this comment.
The RVA check I previously introduced should be sufficient to address the malformations. In general, it does not result in an infinite loop. However, there might be instances where it could raise slice bound errors. Typically, the TLS callback array is located within the data sections—a common characteristic across multiple PECOFF compilers—and the data section is well-aligned with zeros. In my experience handling over 10,000 to 20,000 unique PECOFF binaries, I have not encountered any infinite loops with this implementation. Nonetheless, I believe setting a reasonable limit would be prudent. Is this the solution you were seeking?
There was a problem hiding this comment.
Yea I was just making sure there wasn't some obvious termination condition we could add, e.g., end of file, etc., that would guarantee we are done looping; my general reflex when i see loop is to ask how it can terminate, as we've had bugs in past of infinite loops without proper termination condition, but your answer is sufficient, thank you!
|
note: breaking change |
Parses TLS (thread local storage) data directory in PE64/32.