On 13/02/2023 15:48, Michael Kubacki wrote:
@@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable (
//
// Make sure not to access memory beyond SmbiosEnd
//
- if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) ||
- (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw))
- {
+ Status = SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE),
&SafeIntResult);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((SafeIntResult > (UINTN)SmbiosEnd.Raw) || (SafeIntResult <
(UINTN)Smbios.Raw)) {
return EFI_INVALID_PARAMETER;
}
Could this not be expressed much more cleanly as just:
if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) {
return EFI_INVALID_PARAMETER;
}
without the need to hide a basic arithmetic operation behind an ugly
wrapper function and drag in an external library?
The almost-identical check performed in the code immediately below (that
takes Smbios.Hdr->Length into account) should also be updated to e.g.:
if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < (Smbios.Hdr->Length + 2)) {
...
}
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100116): https://edk2.groups.io/g/devel/message/100116
Mute This Topic: https://groups.io/mt/96938294/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
