Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, April 3, 2018 10:52 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: [PATCH 03/13] 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 <jiaxin.wu@intel.com>
> Cc: Siyuan Fu <siyuan.fu@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=915
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 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