9elements/converged-security-suite

bug: for AMD, need to check that magic bytes are not found randomly

Closed this issue · 6 comments

Here is an excerpt from a Lenovo T14 Gen1 (AMD):

0004a320: 14a0 00f0 edf9 6f20 f0e7 2248 0021 02aa  ......o .."H.!..
0004a330: 0068 16e0 02eb 0113 5b6a 8342 10d1 02eb  .h......[j.B....
0004a340: 0110 1821 806a 2060 0498 01eb 0011 02f1  ...!.j `........
0004a350: 0800 f9f7 85fc 0399 8142 05d1 0020 d5e7  .........B... ..
0004a360: 491c 049b 8b42 e5d8 7220 cfe7 b4e2 0000  I....B..r ......
0004a370: 2442 4844 4249 4f53 2044 4952 2063 6865  $BHDBIOS DIR che
0004a380: 636b 696e 6720 6661 696c 6564 2c20 4249  cking failed, BI
0004a390: 4f53 2069 6d61 6765 2068 6173 2069 6e63  OS image has inc
0004a3a0: 6f72 7265 6374 2061 6464 7265 7373 200a  orrect address .
0004a3b0: 0000 0000 c4e2 0000 feb5 0025 0746 cb21  ...........%.F.!
0004a3c0: 2a46 2846 0095 0195 02f0 3afa 3649 37a0  *F(F......:.6I7.
0004a3d0: 00f0 e8f9 394e 012f 394c 14d0 022f 6846  ....9N./9L.../hF
0004a3e0: 01a9 0ad0 fef7 faff 0700 1dd0 0122 0e21  .............".!
0004a3f0: 0020 02f0 25fa 3846 febd 00f0 6df8 10e0  . ..%.8F....m...
0004a400: 0122 0d21 27e0 2d49 5022 0520 02ab fcf7  .".!'.-IP". ....
0004a410: 8dff 30b9 306c 0090 706c 0190 b06c 2060  ..0.0l..pl...l `
0004a420: 2846 0400 ecd1 1be0 d6e9 0c01 0843 03d1  (F...........C..
0004a430: 2068 10f5 803f 13d1 2068 5022 b064 0098   h...?.. hP".d..

Having the $BHD cookie, 0x0004a370 is falsely recognized by CSS. Then, it OOMs attempting to allocate memory in https://github.com/9elements/converged-security-suite/blob/9ce9f84/pkg/amd/manifest/bios_directory_table.go#L154.
This is because (in the example) it thought to have found millions of entries.

Is there anything that we can use to verify the table to be valid? There is a checksum, so that should tell, I think.

From my debug output:

BIOS Cookie: 0x44484224 ($BHD)
Checksum: 1397705026
Total Entries: 1380533280
Type  | RegionType | ResetImage | CopyImage | ReadOnly | Compressed | Instance | Subprogram | RomID | Size   | SourceAddress | DestinationAddress
----------------------------------------------------------------------------------------------------------------------------------------------------------------

Unfortunately, as per https://doc.coreboot.org/soc/amd/psp_integration.html?highlight=checksum#bios-directory-table-header the checksum includes the entries. I suggest setting a limit to 2000 entries (why is that a 4 byte value anyway? Probably just so it fits nicely in a 32 bit registee...) - and return an error if number of entries > 2000. In a next step, validate the checksum. WDYT?

Let me try to fix it

Will fix it in fiano

If this issue is resolved to satisfaction of @orangecms, may he be so kind and close the issue?

indeed, yea - thank you :)