From 5caaebdc3d3424959b176182b2ecde556a16f554 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 2 Feb 2018 14:38:04 -0800 Subject: [PATCH] adb: restore packet data length checks. These checks were moved to after the read of the payload, which is too late. Add a check before each read to avoid a heap buffer overflow. Test: python test_device.py with x86_64 emulator, walleye Change-Id: I86bcfaaa9004951cc52ad89af74680cf748e717d --- adb/transport.cpp | 5 +++++ adb/transport_usb.cpp | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/adb/transport.cpp b/adb/transport.cpp index 5acaaece6..c90c59c6f 100644 --- a/adb/transport.cpp +++ b/adb/transport.cpp @@ -72,6 +72,11 @@ bool FdConnection::Read(apacket* packet) { return false; } + if (packet->msg.data_length > sizeof(packet->data)) { + D("remote local: read overflow (data length = %" PRIu32 ")", packet->msg.data_length); + return false; + } + if (!ReadFdExactly(fd_.get(), &packet->data, packet->msg.data_length)) { D("remote local: terminated (data)"); return false; diff --git a/adb/transport_usb.cpp b/adb/transport_usb.cpp index 73e8e15df..a1086999d 100644 --- a/adb/transport_usb.cpp +++ b/adb/transport_usb.cpp @@ -61,6 +61,10 @@ static int UsbReadMessage(usb_handle* h, amessage* msg) { static int UsbReadPayload(usb_handle* h, apacket* p) { D("UsbReadPayload(%d)", p->msg.data_length); + if (p->msg.data_length > sizeof(p->data)) { + return -1; + } + #if CHECK_PACKET_OVERFLOW size_t usb_packet_size = usb_get_max_packet_size(h); CHECK_EQ(0ULL, sizeof(p->data) % usb_packet_size); @@ -116,6 +120,11 @@ static int remote_read(apacket* p, usb_handle* usb) { } if (p->msg.data_length) { + if (p->msg.data_length > sizeof(p->data)) { + PLOG(ERROR) << "remote usb: read overflow (data length = " << p->msg.data_length << ")"; + return -1; + } + if (usb_read(usb, p->data, p->msg.data_length)) { PLOG(ERROR) << "remote usb: terminated (data)"; return -1;