Skip to content

system: fastboot: bound filedump path parsing#3608

Open
Old-Ding wants to merge 1 commit into
apache:masterfrom
Old-Ding:codex/fastboot-bound-filedump-path
Open

system: fastboot: bound filedump path parsing#3608
Old-Ding wants to merge 1 commit into
apache:masterfrom
Old-Ding:codex/fastboot-bound-filedump-path

Conversation

@Old-Ding

@Old-Ding Old-Ding commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • parse the filedump path token before copying it into the fixed PATH_MAX buffer
  • reject empty or overlong filedump paths
  • keep the existing optional offset/size parsing behavior

Testing

  • git diff --check
  • git show --stat --check --format=fuller HEAD

Parse the filedump path token separately before copying it into the fixed PATH_MAX buffer. This avoids overflowing the upload path when a host sends an overlong fastboot oem filedump argument while preserving the existing offset and size parsing behavior.

Signed-off-by: Old-Ding <35417409+Old-Ding@users.noreply.github.com>
return;
}

ret = sscanf(arg, "%s %" PRIdOFF " %zu", ctx->upload_param.u.file.path,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's problem call sscanf

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

target buffer overflow evasion?

@xiaoxiang781216 xiaoxiang781216 Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add %xxs?

ret = sscanf(params, "%" PRIdOFF " %zu",
&ctx->upload_param.u.file.offset, &ctx->upload_param.size);
if (ret != 1 && ret != 3)
if (ret != EOF && ret != 0 && ret != 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ( ret==EOF || ret==0 || ret!=2 ) ?
Failed covnersion when EOF is returned, or zero elements assigned, or not two elements assigned? :-P

return;
}
else if (ret == 1)
else if (ret != 2)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should go up in a separate check after file path is parsed? :-)

@cederom cederom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Old-Ding, please note remarks, also please update git commit signature to valid email :-)

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.

3 participants