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.31; helo=mga06.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 B3D262194D3B9 for ; Mon, 15 Oct 2018 22:42:28 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Oct 2018 22:42:28 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,387,1534834800"; d="scan'208";a="81760163" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.11]) by orsmga008.jf.intel.com with ESMTP; 15 Oct 2018 22:42:27 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Star Zeng , Jiewen Yao Date: Tue, 16 Oct 2018 13:43:26 +0800 Message-Id: <20181016054335.47460-4-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 In-Reply-To: <20181016054335.47460-1-ruiyu.ni@intel.com> References: <20181016054335.47460-1-ruiyu.ni@intel.com> Subject: [PATCH v2 03/12] 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: Tue, 16 Oct 2018 05:42:28 -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 | 55 +++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c index 061e4622e8..70442c57da 100644 --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c @@ -177,6 +177,18 @@ 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)) { + DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, total length = %d!\n", Len)); + return NULL; } // @@ -184,24 +196,43 @@ 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; + } + } + + // + // Head->Len is invalid resulting data beyond boundary, or + // Descriptor cannot be found: No such type. + // + if (Len < Offset) { + DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Offset/Len = %d/%d!\n", Offset, Len)); } - if ((Len <= Offset) || (Len < Offset + Head->Len) || - (Head->Type != Type) || (Head->Len < DescLen)) { - DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n")); + if ((Head->Type != Type) || (Head->Len < DescLen)) { + DEBUG ((DEBUG_ERROR, "UsbCreateDesc: descriptor cannot be found, Header(T/L) = %d/%d!\n", Head->Type, Head->Len)); return NULL; } @@ -212,7 +243,7 @@ UsbCreateDesc ( CopyMem (Desc, Head, (UINTN) DescLen); - *Consumed = Offset + Head->Len; + *Consumed = Offset; return Desc; } -- 2.16.1.windows.1