Skip to content

UefiHandler: fix off-by-one OOB read in ParseDepedencyExpression (backport 7-Zip 26.01) - #259

Open
tonghuaroot wants to merge 1 commit into
p7zip-project:masterfrom
tonghuaroot:fix/uefi-handler-off-by-one-backport-26.01
Open

tonghuaroot wants to merge 1 commit into
p7zip-project:masterfrom
tonghuaroot:fix/uefi-handler-off-by-one-backport-26.01

Conversation

@tonghuaroot

Copy link
Copy Markdown

Backports a one-line off-by-one fix landed silently in upstream 7-Zip 26.01.

Bug

ParseDepedencyExpression at CPP/7zip/Archive/UefiHandler.cpp:399:

unsigned command = p[i++];
if (command > ARRAY_SIZE(kExpressionCommands))
  return false;
res += kExpressionCommands[command];

The bound uses > instead of >=. When command == ARRAY_SIZE(kExpressionCommands), the check passes and the next line reads kExpressionCommands[command], which is one element past the end of the static const array. The result then feeds AString::operator+= (const char *), which calls strlen on whatever bytes follow the array in .rodata. Depending on the build, this leaks adjacent constant data or hits unmapped memory and crashes the extractor.

Fix

Tighten the bound to >=, matching the upstream 26.01 source:

-    if (command > ARRAY_SIZE(kExpressionCommands))
+    if (command >= ARRAY_SIZE(kExpressionCommands))

(Replaces the previously-closed PR #255 which had a polluted base.)

ParseDepedencyExpression validates the dependency-expression opcode
with > instead of >=, allowing command == ARRAY_SIZE(kExpressionCommands)
to pass and then dereference kExpressionCommands[command], which is one
element past the end of the static const array. The OOB load is then
passed to AString::operator+=(const char *) via strlen on whatever
bytes follow the array in the binary, potentially leaking adjacent
.rodata content or crashing on unmapped memory.

The upstream 7-Zip 26.01 release tightened this check from > to >=.

Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>
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