summaryrefslogtreecommitdiff
path: root/SecurityPkg
diff options
context:
space:
mode:
authorLaszlo Ersek <lersek@redhat.com>2014-11-14 13:47:14 +0000
committerlersek <lersek@Edk2>2014-11-14 13:47:14 +0000
commitd547f31c32d72e68a3611f7c6db6a8236fe56c0f (patch)
tree83f93d10c3b7803eb4b6a9f775261f4de9a9c65f /SecurityPkg
parentafc18ead283886bfe4b3e21d0a315aead31228af (diff)
downloadedk2-platforms-d547f31c32d72e68a3611f7c6db6a8236fe56c0f.tar.xz
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 <lersek@redhat.com> Reviewed-by: Qin Long <qin.long@intel.com> git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@16389 6f19259b-4bc3-4df7-8a09-765794883524
Diffstat (limited to 'SecurityPkg')
-rw-r--r--SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c7
1 files changed, 5 insertions, 2 deletions
diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 432531f6df..ac043d9a17 100644
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -3186,8 +3186,11 @@ VariableServiceSetVariable (
} 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);