Revision: 16389
          http://sourceforge.net/p/edk2/code/16389
Author:   lersek
Date:     2014-11-14 13:47:14 +0000 (Fri, 14 Nov 2014)
Log Message:
-----------
SecurityPkg: VariableServiceSetVariable(): fix dbt <-> GUID association

SVN r16380 ("UEFI 2.4 X509 Certificate Hash and RFC3161 Timestamp
Verification support for Secure Boot") broke the "dbt" variable's
association with its expected namespace GUID.

According to "MdePkg/Include/Guid/ImageAuthentication.h", *all* of the
"db", "dbx", and "dbt" (== EFI_IMAGE_SECURITY_DATABASE2) variables have
their special meanings in the EFI_IMAGE_SECURITY_DATABASE_GUID namespace.

However, the above commit introduced the following expression in
VariableServiceSetVariable():

> -  } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&
> -          ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || 
> (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))) {
> +  } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&
> +          ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || 
> (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))
> +           || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2)) == 0) {

Simply replacing the individual expressions with the predicates
"GuidMatch", "DbMatch", "DbxMatch", and "DbtMatch", the above
transformation becomes:

> -  } else if (GuidMatch &&
> -          ((DbMatch) || (DbxMatch))) {
> +  } else if (GuidMatch &&
> +          ((DbMatch) || (DbxMatch))
> +           || DbtMatch) {

In shorter form, we change

  GuidMatch && (DbMatch || DbxMatch)

into

  GuidMatch && (DbMatch || DbxMatch) || DbtMatch

which is incorrect, because this way "dbt" will match outside of the
intended namespace / GUID.

The error was caught by gcc:

> SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c: In function
> 'VariableServiceSetVariable':
>
> SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c:3188:71: error:
> suggest parentheses around '&&' within '||' [-Werror=parentheses]
>
>    } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&
>                                                                        ^
> cc1: all warnings being treated as errors

Fix the parentheses.

This change may have security implications.

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Laszlo Ersek <[email protected]>

Reviewed-by: Qin Long <[email protected]>

Revision Links:
--------------
    http://sourceforge.net/p/edk2/code/16380

Modified Paths:
--------------
    trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c

Modified: trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
===================================================================
--- trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c  
2014-11-14 10:24:33 UTC (rev 16388)
+++ trunk/edk2/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c  
2014-11-14 13:47:14 UTC (rev 16389)
@@ -3186,8 +3186,11 @@
   } else if (CompareGuid (VendorGuid, &gEfiGlobalVariableGuid) && (StrCmp 
(VariableName, EFI_KEY_EXCHANGE_KEY_NAME) == 0)) {
     Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, 
&Variable, Attributes, FALSE);
   } else if (CompareGuid (VendorGuid, &gEfiImageSecurityDatabaseGuid) &&
-          ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) || 
(StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0))
-           || (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2)) == 0) {
+             ((StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE)  == 0) ||
+              (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE1) == 0) ||
+              (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2) == 0)
+             )
+            ) {
     Status = ProcessVarWithPk (VariableName, VendorGuid, Data, DataSize, 
&Variable, Attributes, FALSE);
     if (EFI_ERROR (Status)) {
       Status = ProcessVarWithKek (VariableName, VendorGuid, Data, DataSize, 
&Variable, Attributes);


------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-commits mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-commits

Reply via email to