Reviewed-by: Fu Siyuan <siyuan...@intel.com>
> -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Tuesday, April 3, 2018 10:52 PM > To: edk2-devel-01 <edk2-devel@lists.01.org> > Cc: Wu, Jiaxin <jiaxin...@intel.com>; Fu, Siyuan <siyuan...@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...@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> > --- > 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
