diff options
| author | Laszlo Ersek <lersek@redhat.com> | 2014-11-14 13:47:14 +0000 |
|---|---|---|
| committer | lersek <lersek@Edk2> | 2014-11-14 13:47:14 +0000 |
| commit | d547f31c32d72e68a3611f7c6db6a8236fe56c0f (patch) | |
| tree | 83f93d10c3b7803eb4b6a9f775261f4de9a9c65f /SecurityPkg | |
| parent | afc18ead283886bfe4b3e21d0a315aead31228af (diff) | |
| download | edk2-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.c | 7 |
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);
|