From mboxrd@z Thu Jan  1 00:00:00 1970
Subject: [PATCH] SecurityPkg/Library/HashLibBaseCryptoRouter: Fix NULL_RETURNS Coverity issue
To: devel@edk2.groups.io
From: "Ranbir Singh" <Ranbir.Singh3@Dell.com>
X-Originating-Location: Bengaluru, Karnataka, IN (122.172.85.38)
X-Originating-Platform: Windows Chrome 108
User-Agent: GROUPS.IO Web Poster
MIME-Version: 1.0
Date: Wed, 04 Jan 2023 01:02:16 -0800
Message-ID: <Xwgf.1672822936714289617.bJeL@groups.io>
Content-Type: multipart/alternative; boundary="S6FY69ikGJnfR4yTSY3u"

--S6FY69ikGJnfR4yTSY3u
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

In file SecurityPkg/Library/HashLibBaseCryptoRouter/
HashLibBaseCryptoRouterPei.c, the function
CheckSupportedHashMaskMismatch calls InternalGetHashInterfaceHob and
stores return value in HashInterfaceHobLast. Thereafter, it does

ASSERT (HashInterfaceHobLast !=3D NULL);

but this comes into play only in DEBUG mode. In Release mode, the
code continues to proceed to dereferencing "HashInterfaceHobLast"
which will lead to CRASH if HashInterfaceHobLast is NULL.

Hence, for safety add HashInterfaceHobLast NULL pointer check before
accessing further field values.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4225
Signed-off-by: Ranbir Singh <Ranbir.Singh3@Dell.com>
---
.../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c=C2=A0 =C2=A0 =C2=
=A0 =C2=A0| 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoR=
outerPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoR=
outerPei.c
index eeb424b6c3..0c8315ed03 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPe=
i.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPe=
i.c
@@ -108,7 +108,8 @@ CheckSupportedHashMaskMismatch (
HashInterfaceHobLast =3D InternalGetHashInterfaceHob (&gZeroGuid);
ASSERT (HashInterfaceHobLast !=3D NULL);

-=C2=A0 if ((HashInterfaceHobLast->SupportedHashMask !=3D 0) &&
+=C2=A0 if ((HashInterfaceHobLast !=3D NULL) &&
+=C2=A0 =C2=A0 =C2=A0 (HashInterfaceHobLast->SupportedHashMask !=3D 0) &&
(HashInterfaceHobCurrent->SupportedHashMask !=3D HashInterfaceHobLast->Supp=
ortedHashMask))
{
DEBUG ((
--
2.36.1.windows.1

--S6FY69ikGJnfR4yTSY3u
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

<div>In file SecurityPkg/Library/HashLibBaseCryptoRouter/</div>
<div>HashLibBaseCryptoRouterPei.c, the function</div>
<div>CheckSupportedHashMaskMismatch calls InternalGetHashInterfaceHob and</=
div>
<div>stores return value in HashInterfaceHobLast. Thereafter, it does</div>
<div>&nbsp;</div>
<div>&nbsp; ASSERT (HashInterfaceHobLast !=3D NULL);</div>
<div>&nbsp;</div>
<div>but this comes into play only in DEBUG mode. In Release mode, the</div=
>
<div>code continues to proceed to dereferencing "HashInterfaceHobLast"</div=
>
<div>which will lead to CRASH if HashInterfaceHobLast is NULL.</div>
<div>&nbsp;</div>
<div>Hence, for safety add HashInterfaceHobLast NULL pointer check before</=
div>
<div>accessing further field values.</div>
<div>&nbsp;</div>
<div>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4225</div>
<div>Signed-off-by: Ranbir Singh &lt;Ranbir.Singh3@Dell.com&gt;</div>
<div>---</div>
<div>&nbsp;.../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c&nbsp; &=
nbsp; &nbsp; &nbsp;| 3 ++-</div>
<div>&nbsp;1 file changed, 2 insertions(+), 1 deletion(-)</div>
<div>&nbsp;</div>
<div>diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCr=
yptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCr=
yptoRouterPei.c</div>
<div>index eeb424b6c3..0c8315ed03 100644</div>
<div>--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou=
terPei.c</div>
<div>+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRou=
terPei.c</div>
<div>@@ -108,7 +108,8 @@ CheckSupportedHashMaskMismatch (</div>
<div>&nbsp; &nbsp;HashInterfaceHobLast =3D InternalGetHashInterfaceHob (&am=
p;gZeroGuid);</div>
<div>&nbsp; &nbsp;ASSERT (HashInterfaceHobLast !=3D NULL);</div>
<div>&nbsp;</div>
<div>-&nbsp; if ((HashInterfaceHobLast-&gt;SupportedHashMask !=3D 0) &amp;&=
amp;</div>
<div>+&nbsp; if ((HashInterfaceHobLast !=3D NULL) &amp;&amp;</div>
<div>+&nbsp; &nbsp; &nbsp; (HashInterfaceHobLast-&gt;SupportedHashMask !=3D=
 0) &amp;&amp;</div>
<div>&nbsp; &nbsp; &nbsp; &nbsp;(HashInterfaceHobCurrent-&gt;SupportedHashM=
ask !=3D HashInterfaceHobLast-&gt;SupportedHashMask))</div>
<div>&nbsp; &nbsp;{</div>
<div>&nbsp; &nbsp; &nbsp;DEBUG ((</div>
<div>--</div>
<div>2.36.1.windows.1</div>

--S6FY69ikGJnfR4yTSY3u--