[PATCH v2 04/12] CryptoPkg: Fix conditionally uninitialized variable


Michael Kubacki
 

From: Michael Kubacki <michael.kubacki@...>

Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Checks the return value from `ASN1_get_object()` to verify values
set by the function are valid.

Note that the function returns literal `0x80`:
`return (0x80);`

That is used to check the return value is as the case in other areas
of the code.

Cc: Erich McMillan <emcmillan@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Xiaoyu Lu <xiaoyu1.lu@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 21 +++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c b/CryptoPkg/Li=
brary/BaseCryptLib/Pk/CryptX509.c
index 2333157e0d17..1182323b63ee 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -807,6 +807,7 @@ X509GetTBSCert (
UINT32 Asn1Tag;
UINT32 ObjClass;
UINTN Length;
+ UINTN Inf;
=20
//
// Check input parameters.
@@ -836,9 +837,9 @@ X509GetTBSCert (
//
Temp =3D Cert;
Length =3D 0;
- ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int *)&ObjC=
lass, (long)CertSize);
+ Inf =3D ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (=
int *)&ObjClass, (long)CertSize);
=20
- if (Asn1Tag !=3D V_ASN1_SEQUENCE) {
+ if (((Inf & 0x80) =3D=3D 0x00) && (Asn1Tag !=3D V_ASN1_SEQUENCE)) {
return FALSE;
}
=20
@@ -848,7 +849,7 @@ X509GetTBSCert (
//
// Verify the parsed TBSCertificate is one correct SEQUENCE data.
//
- if (Asn1Tag !=3D V_ASN1_SEQUENCE) {
+ if (((Inf & 0x80) =3D=3D 0x00) && (Asn1Tag !=3D V_ASN1_SEQUENCE)) {
return FALSE;
}
=20
@@ -1888,18 +1889,20 @@ Asn1GetTag (
IN UINT32 Tag
)
{
- UINT8 *PtrOld;
- INT32 ObjTag;
- INT32 ObjCls;
- long ObjLength;
+ UINT8 *PtrOld;
+ INT32 ObjTag;
+ INT32 ObjCls;
+ long ObjLength;
+ UINT32 Inf;
=20
//
// Save Ptr position
//
PtrOld =3D *Ptr;
=20
- ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag, &ObjCls, (I=
NT32)(End - (*Ptr)));
- if ((ObjTag =3D=3D (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
+ Inf =3D ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag, &Ob=
jCls, (INT32)(End - (*Ptr)));
+ if (((Inf & 0x80) =3D=3D 0x00) &&
+ (ObjTag =3D=3D (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
(ObjCls =3D=3D (INT32)(Tag & CRYPTO_ASN1_TAG_CLASS_MASK)))
{
*Length =3D (UINTN)ObjLength;
--=20
2.28.0.windows.1


Yao, Jiewen
 

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Wednesday, November 30, 2022 1:33 AM
To: devel@edk2.groups.io
Cc: Erich McMillan <emcmillan@...>; Jiang, Guomin
<guomin.jiang@...>; Wang, Jian J <jian.j.wang@...>; Yao,
Jiewen <jiewen.yao@...>; Michael Kubacki
<mikuback@...>; Lu, Xiaoyu1 <xiaoyu1.lu@...>
Subject: [PATCH v2 04/12] CryptoPkg: Fix conditionally uninitialized
variable

From: Michael Kubacki <michael.kubacki@...>

Fixes CodeQL alerts for CWE-457:
https://cwe.mitre.org/data/definitions/457.html

Checks the return value from `ASN1_get_object()` to verify values
set by the function are valid.

Note that the function returns literal `0x80`:
`return (0x80);`

That is used to check the return value is as the case in other areas
of the code.

Cc: Erich McMillan <emcmillan@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Michael Kubacki <mikuback@...>
Cc: Xiaoyu Lu <xiaoyu1.lu@...>
Co-authored-by: Erich McMillan <emcmillan@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 21 +++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 2333157e0d17..1182323b63ee 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -807,6 +807,7 @@ X509GetTBSCert (
UINT32 Asn1Tag;
UINT32 ObjClass;
UINTN Length;
+ UINTN Inf;

//
// Check input parameters.
@@ -836,9 +837,9 @@ X509GetTBSCert (
//
Temp = Cert;
Length = 0;
- ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int
*)&ObjClass, (long)CertSize);
+ Inf = ASN1_get_object (&Temp, (long *)&Length, (int *)&Asn1Tag, (int
*)&ObjClass, (long)CertSize);

- if (Asn1Tag != V_ASN1_SEQUENCE) {
+ if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {
return FALSE;
}

@@ -848,7 +849,7 @@ X509GetTBSCert (
//
// Verify the parsed TBSCertificate is one correct SEQUENCE data.
//
- if (Asn1Tag != V_ASN1_SEQUENCE) {
+ if (((Inf & 0x80) == 0x00) && (Asn1Tag != V_ASN1_SEQUENCE)) {
return FALSE;
}

@@ -1888,18 +1889,20 @@ Asn1GetTag (
IN UINT32 Tag
)
{
- UINT8 *PtrOld;
- INT32 ObjTag;
- INT32 ObjCls;
- long ObjLength;
+ UINT8 *PtrOld;
+ INT32 ObjTag;
+ INT32 ObjCls;
+ long ObjLength;
+ UINT32 Inf;

//
// Save Ptr position
//
PtrOld = *Ptr;

- ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag, &ObjCls,
(INT32)(End - (*Ptr)));
- if ((ObjTag == (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
+ Inf = ASN1_get_object ((CONST UINT8 **)Ptr, &ObjLength, &ObjTag,
&ObjCls, (INT32)(End - (*Ptr)));
+ if (((Inf & 0x80) == 0x00) &&
+ (ObjTag == (INT32)(Tag & CRYPTO_ASN1_TAG_VALUE_MASK)) &&
(ObjCls == (INT32)(Tag & CRYPTO_ASN1_TAG_CLASS_MASK)))
{
*Length = (UINTN)ObjLength;
--
2.28.0.windows.1