Hi All, I recently encountered an ASSERT in the GetElementsFromRequest function in HiiDatabaseDxe that caused me some vexation. Specifically, this function ASSERTs (TmpRequest != NULL) if L"PATH=" is not found in a ConfigRequest string.
The problem here is that it's possible for code that is remote from this function to trigger that assert via a call to EFI_HII_CONFIG_ACCESS_PROTOCOL->ExtractConfig () with a valid pointer to a garbage request string (or a request string with no L"PATH=" in it). In a debug build, this is troubling, as it's hard to figure out which call to ExtractConfig failed without unwinding the stack or instrumenting every call. However, in a release build, the ASSERT (of course) falls through, and the very next line does two StrStr (NULL, L"...") while evaluating a condition. This looks like a NULL pointer reference at runtime to me, particularly after reviewing StrStr in MdePkg/Library/BaseLib/String.c. Maybe instead of just falling through and doing one or more StrStr (NULL) calls, the release build should return FALSE if TmpRequest == NULL. And maybe the parameter checking in the upstream protocol interface implementations calling GetElementsFromRequest should verify that request strings passed in from external code contain at least one L"PATH=" instance after checking for NULL, and return EFI_INVALID_PARAMETER if they do not, prior to getting anywhere near calling GetElementsFromRequest. Regards, -Ken. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62711): https://edk2.groups.io/g/devel/message/62711 Mute This Topic: https://groups.io/mt/75553629/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
