From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EEBFA21167462 for ; Sun, 14 Oct 2018 23:37:33 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Oct 2018 23:37:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,383,1534834800"; d="scan'208";a="78046738" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.11]) by fmsmga007.fm.intel.com with ESMTP; 14 Oct 2018 23:37:32 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao Date: Mon, 15 Oct 2018 14:38:25 +0800 Message-Id: <20181015063833.61304-4-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 In-Reply-To: <20181015063833.61304-1-ruiyu.ni@intel.com> References: <20181015063833.61304-1-ruiyu.ni@intel.com> Subject: [PATCH 03/11] MdeModulePkg/UsbBus: Fix out-of-bound read access to descriptors X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 15 Oct 2018 06:37:34 -0000 Today's implementation reads the Type/Length field in the USB descriptors data without checking whether the offset to read is beyond the data boundary. The patch fixes this issue. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Star Zeng Cc: Jiewen Yao --- MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 50 ++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c index 061e4622e8..a93060deea 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c @@ -177,6 +177,17 @@ UsbCreateDesc ( DescLen = sizeof (EFI_USB_ENDPOINT_DESCRIPTOR); CtrlLen = sizeof (USB_ENDPOINT_DESC); break; + + default: + ASSERT (FALSE); + return NULL; + } + + // + // Total length is too small that cannot hold the single descriptor header plus data. + // + if (Len <= sizeof (USB_DESC_HEAD)) { + return NULL; } // @@ -184,24 +195,39 @@ UsbCreateDesc ( // format. Skip the descriptor that isn't of this Type // Offset = 0; - Head = (USB_DESC_HEAD*)DescBuf; + Head = (USB_DESC_HEAD *)DescBuf; + while (Offset < Len - sizeof (USB_DESC_HEAD)) { + // + // Above condition make sure Head->Len and Head->Type are safe to access + // + Head = (USB_DESC_HEAD *)&DescBuf[Offset]; - while ((Offset < Len) && (Head->Type != Type)) { - Offset += Head->Len; - if (Len <= Offset) { - DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Beyond boundary!\n")); + if (Head->Len == 0) { + DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n")); return NULL; } - Head = (USB_DESC_HEAD*)(DescBuf + Offset); - if (Head->Len == 0) { - DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n")); + + // + // Make sure no overflow when adding Head->Len to Offset. + // + if (Head->Len > MAX_UINTN - Offset) { + DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = %d!\n", Head->Len)); return NULL; } + + Offset += Head->Len; + + if (Head->Type == Type) { + break; + } } - if ((Len <= Offset) || (Len < Offset + Head->Len) || - (Head->Type != Type) || (Head->Len < DescLen)) { - DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n")); + // + // Head->Len is invalid resulting data beyond boundary, or + // Descriptor cannot be found: No such type. + // + if ((Len < Offset) || (Head->Type != Type) || (Head->Len < DescLen)) { + DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor. Offset/Len = %d/%d, Header(T/L) = %d/%d\n", Offset, Len, Head->Type, Head->Len)); return NULL; } @@ -212,7 +238,7 @@ UsbCreateDesc ( CopyMem (Desc, Head, (UINTN) DescLen); - *Consumed = Offset + Head->Len; + *Consumed = Offset; return Desc; } -- 2.16.1.windows.1