tianocore / edk2

EDK II
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II
Other
4.64k stars 2.51k forks source link

ShellPkg: Fix Optional Data rewriting with bcfg #5971

Closed tormodvolden closed 4 weeks ago

tormodvolden commented 3 months ago

Description

When modifying the Optional Data of a boot option with bcfg boot -opt the result was corrupted data, for instance a concatenation of old data, heap contents, and new data. This was due to a erronous calculation of the original optional data length.

In addition to fixing the calculation, add explaining comments and introduce a helper variable, to not abuse other variables and confuse readers (including the author).

Tagging it "security" because the bug can potentially be used for OOB access.

How This Was Tested

Before:

Shell> bcfg boot -opt 2 ^"abc^"
Shell> bcfg boot dump -v -b    
  ...
  Optional- Y
  00000000: 61 00 62 00 63 00 00 00-                         *a.b.c...*

Shell> bcfg boot -opt 2 ^"def^"
Shell> bcfg boot dump -v -b    
  ...
  Optional- Y
  00000000: 61 00 62 00 63 00 00 00-70 74 61 6C AF AF AF AF  *a.b.c...ptal....*
  00000010: 64 00 65 00 66 00 00 00-                         *d.e.f...*

After:

Shell> bcfg boot -opt 2 ^"abc^"
Shell> bcfg boot dump -v -b    
  ...
  Optional- Y
  00000000: 61 00 62 00 63 00 00 00-                         *a.b.c...*

Shell> bcfg boot -opt 2 ^"def^"
Shell> bcfg boot dump -v -b    
  ...
  Optional- Y
  00000000: 64 00 65 00 66 00 00 00-                         *d.e.f...*

Integration Instructions

N/A

pierregondois commented 1 month ago

Hello @leiflindholm @mdkinney @ajfish , Just a ping in case the PR looks good to you