On August 9, 2021 5:51 PM, Marvin Häuser wrote:
> The current certificate lookup code does not check the bounds of the
> authentication data before accessing it. Abort if the header cannot fit. Also,
> the lookup code aborts once the authetication data is smaller than an
> algorithm's OID size. As OIDs are variably-sized, this may cause unexpected
> authentication failure due to the early error-exit.
>
> Additionally move the two-byte encoding check out of the loop as the data is
> invariant.
>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Jian J Wang <jian.j.w...@intel.com>
> Cc: Min Xu <min.m...@intel.com>
> Cc: Vitaly Cheptsov <vit9...@protonmail.com>
> Signed-off-by: Marvin Häuser <mhaeu...@posteo.de>
> ---
> SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 43
> +++++++++++---------
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index c48861cd6496..6615099baafb 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
> +++ b.c
> @@ -624,30 +624,33 @@ HashPeImageByType ( {
>
> UINT8 Index;
>
>
>
> + if (AuthDataSize < 32) {
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> + //
>
> + // Check the Hash algorithm in PE/COFF Authenticode.
>
> + // According to PKCS#7 Definition:
>
> + // SignedData ::= SEQUENCE {
>
> + // version Version,
>
> + // digestAlgorithms DigestAlgorithmIdentifiers,
>
> + // contentInfo ContentInfo,
>
> + // .... }
>
> + // The DigestAlgorithmIdentifiers can be used to determine the hash
> algorithm in PE/COFF hashing
>
> + // This field has the fixed offset (+32) in final Authenticode ASN.1
> data.
>
> + // Fixed offset (+32) is calculated based on two bytes of length
> encoding.
>
> + //
>
> + if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
>
> + //
>
> + // Only support two bytes of Long Form of Length Encoding.
>
> + //
>
> + return EFI_UNSUPPORTED;
>
> + }
>
> +
>
> for (Index = 0; Index < HASHALG_MAX; Index++) {
>
> - //
>
> - // Check the Hash algorithm in PE/COFF Authenticode.
>
> - // According to PKCS#7 Definition:
>
> - // SignedData ::= SEQUENCE {
>
> - // version Version,
>
> - // digestAlgorithms DigestAlgorithmIdentifiers,
>
> - // contentInfo ContentInfo,
>
> - // .... }
>
> - // The DigestAlgorithmIdentifiers can be used to determine the hash
> algorithm in PE/COFF hashing
>
> - // This field has the fixed offset (+32) in final Authenticode ASN.1
> data.
>
> - // Fixed offset (+32) is calculated based on two bytes of length
> encoding.
>
> - //
>
> - if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
>
> - //
>
> - // Only support two bytes of Long Form of Length Encoding.
>
> - //
>
> + if (AuthDataSize - 32 < mHash[Index].OidLength) {
>
> continue;
>
> }
>
>
>
> - if (AuthDataSize < 32 + mHash[Index].OidLength) {
>
> - return EFI_UNSUPPORTED;
>
> - }
>
> -
>
> if (CompareMem (AuthData + 32, mHash[Index].OidValue,
> mHash[Index].OidLength) == 0) {
>
> break;
>
> }
>
> --
> 2.31.1
Reviewed-by: Min Xu <min.m...@intel.com>
Thanks!
Xu, Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79129): https://edk2.groups.io/g/devel/message/79129
Mute This Topic: https://groups.io/mt/84764903/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
