From 44eb974081ce6abb98fb82ec35b77d790f48dda3 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Sat, 31 Mar 2018 16:04:10 +0200 Subject: [PATCH] NetworkPkg/TlsDxe: verify DataSize for EfiTlsCipherList TlsSetSessionData() shouldn't just ignore an incomplete EFI_TLS_CIPHER element at the end of "Data": - Generally speaking, malformed input for a security API is best rejected explicitly. - Specifically speaking, the size of EFI_TLS_CIPHER is 2 bytes. If DataSize is 1 on input, then the initial check for (DataSize == 0) will fail, but then TlsSetCipherList() will be called with CipherNum=0. Return EFI_INVALID_PARAMETER from TlsSetSessionData() if "Data" doesn't contain a whole number of EFI_TLS_CIPHER elements. While at it, introduce the dedicated variable CipherCount. Cc: Jiaxin Wu Cc: Siyuan Fu Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek Reviewed-by: Fu Siyuan Reviewed-by: Long Qin Reviewed-by: Jiaxin Wu --- NetworkPkg/TlsDxe/TlsProtocol.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/NetworkPkg/TlsDxe/TlsProtocol.c b/NetworkPkg/TlsDxe/TlsProtocol.c index ad4c922c60..a5f95a0983 100644 --- a/NetworkPkg/TlsDxe/TlsProtocol.c +++ b/NetworkPkg/TlsDxe/TlsProtocol.c @@ -38,6 +38,7 @@ EFI_TLS_PROTOCOL mTlsProtocol = { This is NULL. Data is NULL. DataSize is 0. + DataSize is invalid for DataType. @retval EFI_UNSUPPORTED The DataType is unsupported. @retval EFI_ACCESS_DENIED If the DataType is one of below: EfiTlsClientRandom @@ -59,6 +60,7 @@ TlsSetSessionData ( EFI_STATUS Status; TLS_INSTANCE *Instance; UINT16 *CipherId; + UINTN CipherCount; UINTN Index; EFI_TPL OldTpl; @@ -100,17 +102,23 @@ TlsSetSessionData ( Status = TlsSetConnectionEnd (Instance->TlsConn, *((EFI_TLS_CONNECTION_END *) Data)); break; case EfiTlsCipherList: + if (DataSize % sizeof (EFI_TLS_CIPHER) != 0) { + Status = EFI_INVALID_PARAMETER; + goto ON_EXIT; + } + CipherId = AllocatePool (DataSize); if (CipherId == NULL) { Status = EFI_OUT_OF_RESOURCES; goto ON_EXIT; } - for (Index = 0; Index < DataSize / sizeof (EFI_TLS_CIPHER); Index++) { + CipherCount = DataSize / sizeof (EFI_TLS_CIPHER); + for (Index = 0; Index < CipherCount; Index++) { *(CipherId +Index) = HTONS (*(((UINT16 *) Data) + Index)); } - Status = TlsSetCipherList (Instance->TlsConn, CipherId, DataSize / sizeof (EFI_TLS_CIPHER)); + Status = TlsSetCipherList (Instance->TlsConn, CipherId, CipherCount); FreePool (CipherId); break; -- 2.39.2