edk2 icon indicating copy to clipboard operation
edk2 copied to clipboard

ShellPkg: Fix bug #3080, OOB, minor UefiShellLib fixes

Open tormodvolden opened this issue 7 months ago • 2 comments

Description

The commit Prevent out-of-bounds access fixes a possible OOB in a way that is already used in another place in the file.

The commits Correct check for empty string and Simplify check for empty string can be squashed if the maintainers prefer, for the sake of review they are split up in a fix-up and an optimization.

The commit Only write value if successful conversion is for correctness and conformance to documented behavior.

The commit Accept "0 " as valid numeric string fixes a long-standing (since 2011?) bug that prevented the use of "0" as a numeric argument, e.g. to address the first boot option when using bcfg. A workaround, known to some, was to use "0x0" instead.

Note there was a similar fix for InternalShellStrDecimalToUint64() in commit 80f3e34f, however if ShellConvertStringToUint64() is called with ForceHex=TRUE, InternalShellStrHexToUint64() is called instead so it wouldn't help.

  • [ ] Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • [x] Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • [ ] Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

The fix for bug 3080 was tested with builds from OvmfPkg in QEMU. Before:

Shell> fs0:
FS0:\> bcfg boot -opt 0 FS0:\options.txt
bcfg: Invalid argument - 'Option Index'
FS0:\> 

After:

Shell> fs0:
FS0:\> bcfg boot -opt 0 FS0:\options.txt
FS0:\> 

Integration Instructions

N/A

tormodvolden avatar Jul 23 '24 23:07 tormodvolden