Branch: refs/heads/security-advisory/cve-2024-38797/advisory
Home: https://github.com/tianocore/edk2
Commit: fc8d89304b16ade5368dcf5af2a87e3950328147
https://github.com/tianocore/edk2/commit/fc8d89304b16ade5368dcf5af2a87e3950328147
Author: Doug Flick <dougfl...@microsoft.com>
Date: 2025-04-07 (Mon, 07 Apr 2025)
Changed paths:
M SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
Log Message:
-----------
SecurityPkg: Out of bound read in HashPeImageByType()
In HashPeImageByType(), the hash of PE/COFF image is calculated.
This function may get untrusted input.
Inside this function, the following code verifies the loaded image has
the correct format, by reading the second byte of the buffer.
```c
if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
...
}
```
The input image is not trusted and that may not have the second byte to
read. So this poses an out of bound read error.
With below fix we are assuring that we don't do out of bound read. i.e,
we make sure that AuthDataSize is greater than 1.
```c
if (AuthDataSize > 1
&& (*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE){
...
}
```
AuthDataSize size is verified before reading the second byte.
So if AuthDataSize is less than 2, the second byte will not be read, and
the out of bound read situation won't occur.
Tested the patch on real platform with and without TPM connected and
verified image is booting fine.
Authored-by: Raj AlwinX Selvaraj <alw...@intel.com>
Signed-off-by: Doug Flick <dougfl...@microsoft.com>
Commit: cc76b551dea7ccdfe767a3287923afe804baa0ff
https://github.com/tianocore/edk2/commit/cc76b551dea7ccdfe767a3287923afe804baa0ff
Author: Doug Flick <dougfl...@microsoft.com>
Date: 2025-04-07 (Mon, 07 Apr 2025)
Changed paths:
M SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
Log Message:
-----------
SecurityPkg: Improving HashPeImageByType () logic
Namely:
(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
to TRUE for Index==0, then it will evaluate to TRUE for all other
Index values as well. As a result, the (Index == HASHALG_MAX)
condition will fire after the loop, and we'll return
EFI_UNSUPPORTED.
While this is correct, functionally speaking, it is wasteful to
keep re-checking TWO_BYTE_ENCODE in the loop body. The check
should be made at the top of the function, and EFI_UNSUPPORTED
should be returned at once, if appropriate.
(2) If the hash algorithm selected by Index has such a large OID that
the OID comparison cannot even be performed (because AuthDataSize
is not large enough for containing the OID in question, starting
at offset 32), then the function returns EFI_UNSUPPORTED at once.
This is bogus; this case should simply be treated as an OID
mismatch, and the loop should advance to the next Index value /
hash algorithm candidate. A remaining hash algo may have a shorter
OID and yield an OID match.
Signed-off-by: Doug Flick <dougfl...@microsoft.com>
squash
Commit: 3bac0b1c9167e7bcf5fa5e850f28beb3f333fdf4
https://github.com/tianocore/edk2/commit/3bac0b1c9167e7bcf5fa5e850f28beb3f333fdf4
Author: Doug Flick <dougfl...@microsoft.com>
Date: 2025-04-07 (Mon, 07 Apr 2025)
Changed paths:
M
SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
Log Message:
-----------
SecurityPkg: Improving SecureBootConfigImpl:HashPeImageByType () logic
Namely:
(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
to TRUE for Index==0, then it will evaluate to TRUE for all other
Index values as well. As a result, the (Index == HASHALG_MAX)
condition will fire after the loop, and we'll return
EFI_UNSUPPORTED.
While this is correct, functionally speaking, it is wasteful to
keep re-checking TWO_BYTE_ENCODE in the loop body. The check
should be made at the top of the function, and EFI_UNSUPPORTED
should be returned at once, if appropriate.
(2) If the hash algorithm selected by Index has such a large OID that
the OID comparison cannot even be performed (because AuthDataSize
is not large enough for containing the OID in question, starting
at offset 32), then the function returns EFI_UNSUPPORTED at once.
This is bogus; this case should simply be treated as an OID
mismatch, and the loop should advance to the next Index value /
hash algorithm candidate. A remaining hash algo may have a shorter
OID and yield an OID match.
Signed-off-by: Doug Flick <dougfl...@microsoft.com>
Compare: https://github.com/tianocore/edk2/compare/1d5063763bed...3bac0b1c9167
To unsubscribe from these emails, change your notification settings at
https://github.com/tianocore/edk2/settings/notifications
_______________________________________________
edk2-commits mailing list
edk2-commits@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-commits
