Fix incorrect bounds check for hfs_thread structs
Fixes #2899. The problem was caused by an incorrect bounds check introduced in #2647.
The hfs_thread struct is sized for the maximum possible name length, but the actual on-disk storage of the name is variable length. It's incorrect to require the buffer to be the maximum size, and results in entries being skipped as corrupt. Instead, the right thing to do is check that enough of the data fits into the buffer so that the name length can be read, and if so, read the name length and check that a name of that length also fits.
Do you have any test data you could share?
We noticed it with a SANS training image. Digital Corpora has https://corp.digitalcorpora.org/corpora/drives/nps-2009-hfsjtest1/, not sure whether that exhibits missing entries in fls/fiwalk. I imagine that most HFS+ images would demonstrate the bug.
#2879 notes the same issue.
We noticed it with a SANS training image. Digital Corpora has https://corp.digitalcorpora.org/corpora/drives/nps-2009-hfsjtest1/, not sure whether that exhibits missing entries in
fls/fiwalk. I imagine that most HFS+ images would demonstrate the bug.
Yea digital corpora! I need to do some testing to see how the various code coverage tools happen if multiple executables get run as part of a single test. I suspect that we can't just run fiwalk repeatedly and get accurate code coverage results.
Can you figure out why this pull request triggered no runs?
Can you figure out why this pull request triggered no runs?
I think it did:
I did.
On Thu, Sep 5, 2024 at 12:30 PM Joel Uckelman @.***> wrote:
Can you figure out why this pull request triggered no runs?
I think it did: image.png (view on web) https://github.com/user-attachments/assets/82376e5e-86f2-43e9-b33a-bc676058c506
— Reply to this email directly, view it on GitHub https://github.com/sleuthkit/sleuthkit/pull/2940#issuecomment-2332172165, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFHLHVRFDDUGL656KTA5LZVCBIPAVCNFSM6AAAAABMMKIDLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZSGE3TEMJWGU . You are receiving this because you commented.Message ID: @.***>
@uckelman-sf - Where are all of your commits coming from ? Your internal fork?
@uckelman-sf - Where are all of your commits coming from ? Your internal fork?
Yes, before they're pushed to my public repo here on GitHub.
We noticed it with a SANS training image. Digital Corpora has https://corp.digitalcorpora.org/corpora/drives/nps-2009-hfsjtest1/, not sure whether that exhibits missing entries in
fls/fiwalk. I imagine that most HFS+ images would demonstrate the bug.
Is the SANS training image publicly available?
s the SANS training image publicly available?
No, but maybe they'd grant permission for them to be shared with you privately (they gave me that permission in the past, but that was a long time ago).
My rough understanding of this HFS+ bug, though, is that it'll probably be exhibited with any normal nontrivial HFS+ image.
Well, let’s make another image that demonstrates the bug then
On Thu, Sep 5, 2024 at 7:14 PM Jon Stewart @.***> wrote:
s the SANS training image publicly available?
No, but maybe they'd grant permission for them to be shared with you privately (they gave me that permission in the past, but that was a long time ago).
My rough understanding of this HFS+ bug, though, is that it'll probably be exhibited with any normal nontrivial HFS+ image.
— Reply to this email directly, view it on GitHub https://github.com/sleuthkit/sleuthkit/pull/2940#issuecomment-2333054701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMFHLAS76AF7JQKUTQKK4LZVEFWPAVCNFSM6AAAAABMMKIDLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZTGA2TINZQGE . You are receiving this because you modified the open/close state.Message ID: @.***>