onekey-sec / unblob

Extract files from any kind of container formats
https://unblob.org
Other
2.16k stars 80 forks source link

fix(ubi): add exception handler for SeekError #852

Closed mucoze closed 3 months ago

mucoze commented 4 months ago

This check is needed to exit the loop when the end of the file is reached.

qkaiser commented 4 months ago

My understanding is that if a SeekError is raised, then the guessed PEB size is wrong or the UBI image is truncated. If the guessed PEB size is wrong, we should raise an InvalidInputFormat rather than silently returning a wrong offset.

What did you observe ? Was it a truncated image or the PEB is wrong ?

mucoze commented 4 months ago

PEB size is correct. I don't think the UBI images are truncated, given that they are valid samples and can also be properly extracted by ubireader_extract_images manually.

I'm observing SeekError on 3 different samples.

e3krisztian commented 3 months ago

I think the changes are fine, and could be merged.

I somewhat miss some means of test, that shows that it is working as intended.

Another potential problem I was thinking on is, when the _walk_ubi exits early due to the change resulting in a too small to be useful ubifs chunk. I think it is not a big issue, as it would fail at extraction time.

Would a minimal size check in _walk_ubi/calculate_chunk be valid and useful here?

qkaiser commented 3 months ago

@e3krisztian integration tests files demonstrating the behavior have been added by @mucoze

Would a minimal size check in _walk_ubi/calculate_chunk be valid and useful here?

Minimal size checks are usually arbitrary and not super useful IMO. As you said, failing case is not a big issue as it would fail at extraction time.