sleuthkit icon indicating copy to clipboard operation
sleuthkit copied to clipboard

Fix incorrect bounds check for hfs_thread structs

Open uckelman-sf opened this issue 1 year ago • 1 comments

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.

uckelman-sf avatar Aug 12 '24 14:08 uckelman-sf

Do you have any test data you could share?

APriestman avatar Aug 13 '24 11:08 APriestman

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.

jonstewart avatar Sep 03 '24 01:09 jonstewart

#2879 notes the same issue.

jonstewart avatar Sep 03 '24 01:09 jonstewart

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.

simsong avatar Sep 05 '24 16:09 simsong

Can you figure out why this pull request triggered no runs?

simsong avatar Sep 05 '24 16:09 simsong

Can you figure out why this pull request triggered no runs?

I think it did: image

uckelman-sf avatar Sep 05 '24 16:09 uckelman-sf

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: @.***>

simsong avatar Sep 05 '24 16:09 simsong

@uckelman-sf - Where are all of your commits coming from ? Your internal fork?

simsong avatar Sep 05 '24 18:09 simsong

@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.

uckelman-sf avatar Sep 05 '24 18:09 uckelman-sf

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?

simsong avatar Sep 05 '24 19:09 simsong

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.

jonstewart avatar Sep 06 '24 02:09 jonstewart

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: @.***>

simsong avatar Sep 06 '24 04:09 simsong