Skip to content

e2fsck: validate fast commit dentry tag length#274

Open
TristanInSec wants to merge 1 commit into
tytso:masterfrom
TristanInSec:fix-fc-dentry-underflow
Open

e2fsck: validate fast commit dentry tag length#274
TristanInSec wants to merge 1 commit into
tytso:masterfrom
TristanInSec:fix-fc-dentry-underflow

Conversation

@TristanInSec

Copy link
Copy Markdown

Summary

In tl_to_darg(), the function copies sizeof(struct ext4_fc_dentry_info) bytes from the tag value buffer without first checking that the tag's data length is at least that large. A crafted filesystem with a truncated EXT4_FC_TAG_LINK, EXT4_FC_TAG_UNLINK, or EXT4_FC_TAG_CREAT tag triggers a heap buffer over-read (and potentially a stack buffer under-read when computing the filename length via subtraction).

Add a bounds check rejecting tags shorter than the fixed-size dentry info header before the memcpy.

Testing

Built and verified the fix compiles cleanly against current master. Confirmed the vulnerable path is reachable by processing a crafted ext4 image with fast commit journal entries.

Add a bounds check to verify that the fast commit tag length is at
least sizeof(struct ext4_fc_dentry_info) before computing dname_len.

When ext4_fc_tag_len(tl) returns a value smaller than sizeof(struct
ext4_fc_dentry_info), the subtraction on line 628 underflows since
dname_len is a signed int. With fc_len=7, dname_len becomes -1, which
causes malloc(0) to return a non-NULL pointer (on glibc), followed by
memcpy with a size of (size_t)-1, resulting in a heap buffer over-read
and crash.

This is reachable when e2fsck replays a journal with
EXT4_FEATURE_INCOMPAT_FAST_COMMIT set and a malformed dentry tag
(EXT4_FC_TAG_CREAT, EXT4_FC_TAG_LINK, or EXT4_FC_TAG_UNLINK).

Return -EINVAL early if the tag is too short, which causes the replay
loop to abort via the existing ret < 0 check.

Signed-off-by: Tristan <tristmd+ai@gmail.com>
@TristanInSec

Copy link
Copy Markdown
Author

Hi, friendly ping on this PR -- it's been open for ~5 weeks. The fix validates fast commit dentry tag length to prevent an integer underflow. Let me know if any changes are needed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant