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 <jiaxin...@intel.com> Cc: Siyuan Fu <siyuan...@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915 Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <ler...@redhat.com> Reviewed-by: Fu Siyuan <siyuan...@intel.com> --- Notes: v2: - pick up Siyuan's R-b 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 ad4c922c60bd..a5f95a098345 100644 --- a/NetworkPkg/TlsDxe/TlsProtocol.c +++ b/NetworkPkg/TlsDxe/TlsProtocol.c @@ -35,12 +35,13 @@ EFI_TLS_PROTOCOL mTlsProtocol = { @retval EFI_SUCCESS The TLS session data is set successfully. @retval EFI_INVALID_PARAMETER One or more of the following conditions is TRUE: 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 EfiTlsServerRandom EfiTlsKeyMaterial @retval EFI_NOT_READY Current TLS session state is NOT @@ -56,12 +57,13 @@ TlsSetSessionData ( IN UINTN DataSize ) { EFI_STATUS Status; TLS_INSTANCE *Instance; UINT16 *CipherId; + UINTN CipherCount; UINTN Index; EFI_TPL OldTpl; Status = EFI_SUCCESS; CipherId = NULL; @@ -97,23 +99,29 @@ TlsSetSessionData ( goto ON_EXIT; } 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; case EfiTlsCompressionMethod: // // TLS seems only define one CompressionMethod.null, which specifies that data exchanged via the -- 2.14.1.3.gb7cf6e02401b _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
