android-tools: Backport fixes for CVEs
CVE-2022-3168 and CVE-2022-20128.
This commit is contained in:
parent
54b84d2383
commit
65d94eab24
|
@ -0,0 +1,47 @@
|
|||
From a547c7001ce5a3f5ff6611086d0c9e2d6b52c6d9 Mon Sep 17 00:00:00 2001
|
||||
From: Shaju Mathew <shaju@google.com>
|
||||
Date: Mon, 17 Jan 2022 17:42:05 -0800
|
||||
Subject: [PATCH] Now suppressing ability for a potentially rogue device to
|
||||
engage in directory traversal on host.
|
||||
|
||||
Bug:209438553
|
||||
|
||||
Ignore-AOSP-First: Resolution for (potential) security exploit if the device daemon
|
||||
happens to be compromised.
|
||||
|
||||
Test: - Manual/cursory test against poc daemon (py script).
|
||||
- For addressing flake: $aosp-master-with-phones/tools/asuite/atest$ atest atest_unittests
|
||||
<snip>
|
||||
Summary
|
||||
-------
|
||||
arm64-v8a atest_unittests: Passed: 288, Failed: 0, Ignored: 0, Assumption Failed: 0 <snip>
|
||||
All tests passed!
|
||||
|
||||
Signed-off-by: Shaju Mathew <shaju@google.com>
|
||||
Change-Id: I3e28b8882a0741a734422c52057d5ad1e608d8a8
|
||||
---
|
||||
client/file_sync_client.cpp | 8 ++++++++
|
||||
1 file changed, 8 insertions(+)
|
||||
|
||||
diff --git a/client/file_sync_client.cpp b/client/file_sync_client.cpp
|
||||
index af1deba1..1b1dfa89 100644
|
||||
--- a/client/file_sync_client.cpp
|
||||
+++ b/vendor/adb/client/file_sync_client.cpp
|
||||
@@ -545,6 +545,14 @@ class SyncConnection {
|
||||
if (!ReadFdExactly(fd, buf, len)) return false;
|
||||
buf[len] = 0;
|
||||
|
||||
+ // Address the highly unlikely scenario wherein a
|
||||
+ // compromised device/service might be able to
|
||||
+ // traverse across directories on the host. Let's
|
||||
+ // shut that door!
|
||||
+ if (strchr(buf, '/')) {
|
||||
+ return false;
|
||||
+ }
|
||||
+
|
||||
callback(dent.mode, dent.size, dent.mtime, buf);
|
||||
}
|
||||
}
|
||||
--
|
||||
2.38.0
|
||||
|
|
@ -0,0 +1,184 @@
|
|||
From 13508c1c97da14a294c04e5097ea81c9ce7edf33 Mon Sep 17 00:00:00 2001
|
||||
From: Shaju Mathew <shaju@google.com>
|
||||
Date: Sat, 25 Jun 2022 14:57:31 +0000
|
||||
Subject: [PATCH] Reject external connect: requests.
|
||||
|
||||
Steps:
|
||||
1. Track forward:reverse config in a data-structure.
|
||||
2. connect_to_remote() examines each socket transport and updates
|
||||
this data-structure.
|
||||
3. handle_packet() takes appropriate action
|
||||
(abort) for an unknown connect: request originating from the device.
|
||||
|
||||
Bug:205286508
|
||||
|
||||
Test: treehugger
|
||||
|
||||
Signed-off-by: jmgao <jmgao@fb.com>
|
||||
Change-Id: I0ec7d6f8e60afc2ee5d1be2b63bf90ca99443a52
|
||||
---
|
||||
adb.cpp | 11 +++++++++-
|
||||
sockets.cpp | 6 +++++
|
||||
transport.cpp | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++-
|
||||
transport.h | 12 ++++++++++
|
||||
4 files changed, 88 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/adb.cpp b/adb.cpp
|
||||
index 5d481295..f397da09 100644
|
||||
--- a/adb.cpp
|
||||
+++ b/vendor/adb/adb.cpp
|
||||
@@ -485,7 +485,16 @@ void handle_packet(apacket *p, atransport *t)
|
||||
// byte. The client sent strings with null termination, which post-string_view, start
|
||||
// being interpreted as part of the string, unless we explicitly strip them.
|
||||
address = StripTrailingNulls(address);
|
||||
-
|
||||
+#if ADB_HOST
|
||||
+ // The incoming address (from the payload) might be some other
|
||||
+ // target (e.g tcp:<ip>:8000), however we do not allow *any*
|
||||
+ // such requests - namely, those from (a potentially compromised)
|
||||
+ // adbd (reverse:forward: source) port transport.
|
||||
+ if (!t->IsReverseConfigured(address.data())) {
|
||||
+ LOG(FATAL) << __func__ << " disallowed connect to " << address << " from "
|
||||
+ << t->serial_name();
|
||||
+ }
|
||||
+#endif
|
||||
asocket* s = create_local_service_socket(address, t);
|
||||
if (s == nullptr) {
|
||||
send_close(0, p->msg.arg0, t);
|
||||
diff --git a/sockets.cpp b/sockets.cpp
|
||||
index 61a2d9d9..3cd43f92 100644
|
||||
--- a/sockets.cpp
|
||||
+++ b/vendor/adb/sockets.cpp
|
||||
@@ -560,6 +560,12 @@ asocket* create_remote_socket(unsigned id, atransport* t) {
|
||||
}
|
||||
|
||||
void connect_to_remote(asocket* s, std::string_view destination) {
|
||||
+#if ADB_HOST
|
||||
+ // Snoop reverse:forward: requests to track them so that an
|
||||
+ // appropriate filter (to figure out whether the remote is
|
||||
+ // allowed to connect locally) can be applied.
|
||||
+ s->transport->UpdateReverseConfig(destination);
|
||||
+#endif
|
||||
D("Connect_to_remote call RS(%d) fd=%d", s->id, s->fd);
|
||||
apacket* p = get_apacket();
|
||||
|
||||
diff --git a/transport.cpp b/transport.cpp
|
||||
index 71771d31..f1cae405 100644
|
||||
--- a/transport.cpp
|
||||
+++ b/vendor/adb/transport.cpp
|
||||
@@ -1219,8 +1219,10 @@ bool atransport::HandleRead(std::unique_ptr<apacket> p) {
|
||||
VLOG(TRANSPORT) << dump_packet(serial.c_str(), "from remote", p.get());
|
||||
apacket* packet = p.release();
|
||||
|
||||
- // TODO: Does this need to run on the main thread?
|
||||
+ // This needs to run on the main thread since the associated fdevent
|
||||
+ // message pump exists in that context.
|
||||
fdevent_run_on_main_thread([packet, this]() { handle_packet(packet, this); });
|
||||
+
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -1614,6 +1616,63 @@ void unregister_usb_transport(usb_handle* usb) {
|
||||
return t->GetUsbHandle() == usb && t->GetConnectionState() == kCsNoPerm;
|
||||
});
|
||||
}
|
||||
+
|
||||
+// Track reverse:forward commands, so that info can be used to develop
|
||||
+// an 'allow-list':
|
||||
+// - adb reverse tcp:<device_port> localhost:<host_port> : responds with the
|
||||
+// device_port
|
||||
+// - adb reverse --remove tcp:<device_port> : responds OKAY
|
||||
+// - adb reverse --remove-all : responds OKAY
|
||||
+void atransport::UpdateReverseConfig(std::string_view service_addr) {
|
||||
+ check_main_thread();
|
||||
+ if (!android::base::ConsumePrefix(&service_addr, "reverse:")) {
|
||||
+ return;
|
||||
+ }
|
||||
+
|
||||
+ if (android::base::ConsumePrefix(&service_addr, "forward:")) {
|
||||
+ // forward:[norebind:]<remote>;<local>
|
||||
+ bool norebind = android::base::ConsumePrefix(&service_addr, "norebind:");
|
||||
+ auto it = service_addr.find(';');
|
||||
+ if (it == std::string::npos) {
|
||||
+ return;
|
||||
+ }
|
||||
+ std::string remote(service_addr.substr(0, it));
|
||||
+
|
||||
+ if (norebind && reverse_forwards_.find(remote) != reverse_forwards_.end()) {
|
||||
+ // This will fail, don't update the map.
|
||||
+ LOG(DEBUG) << "ignoring reverse forward that will fail due to norebind";
|
||||
+ return;
|
||||
+ }
|
||||
+
|
||||
+ std::string local(service_addr.substr(it + 1));
|
||||
+ reverse_forwards_[remote] = local;
|
||||
+ } else if (android::base::ConsumePrefix(&service_addr, "killforward:")) {
|
||||
+ // kill-forward:<remote>
|
||||
+ auto it = service_addr.find(';');
|
||||
+ if (it != std::string::npos) {
|
||||
+ return;
|
||||
+ }
|
||||
+ reverse_forwards_.erase(std::string(service_addr));
|
||||
+ } else if (service_addr == "killforward-all") {
|
||||
+ reverse_forwards_.clear();
|
||||
+ } else if (service_addr == "list-forward") {
|
||||
+ LOG(DEBUG) << __func__ << " ignoring --list";
|
||||
+ } else { // Anything else we need to know about?
|
||||
+ LOG(FATAL) << "unhandled reverse service: " << service_addr;
|
||||
+ }
|
||||
+}
|
||||
+
|
||||
+// Is this an authorized :connect request?
|
||||
+bool atransport::IsReverseConfigured(const std::string& local_addr) {
|
||||
+ check_main_thread();
|
||||
+ for (const auto& [remote, local] : reverse_forwards_) {
|
||||
+ if (local == local_addr) {
|
||||
+ return true;
|
||||
+ }
|
||||
+ }
|
||||
+ return false;
|
||||
+}
|
||||
+
|
||||
#endif
|
||||
|
||||
bool check_header(apacket* p, atransport* t) {
|
||||
diff --git a/transport.h b/transport.h
|
||||
index ed2cd81f..fc0e322d 100644
|
||||
--- a/transport.h
|
||||
+++ b/vendor/adb/transport.h
|
||||
@@ -31,6 +31,7 @@
|
||||
#include <string>
|
||||
#include <string_view>
|
||||
#include <thread>
|
||||
+#include <unordered_map>
|
||||
#include <vector>
|
||||
|
||||
#include <android-base/macros.h>
|
||||
@@ -298,6 +299,10 @@ class atransport : public enable_weak_from_this<atransport> {
|
||||
#if ADB_HOST
|
||||
void SetUsbHandle(usb_handle* h) { usb_handle_ = h; }
|
||||
usb_handle* GetUsbHandle() { return usb_handle_; }
|
||||
+
|
||||
+ // Interface for management/filter on forward:reverse: configuration.
|
||||
+ void UpdateReverseConfig(std::string_view service_addr);
|
||||
+ bool IsReverseConfigured(const std::string& local_addr);
|
||||
#endif
|
||||
|
||||
const TransportId id;
|
||||
@@ -427,6 +432,13 @@ class atransport : public enable_weak_from_this<atransport> {
|
||||
|
||||
bool delayed_ack_ = false;
|
||||
|
||||
+#if ADB_HOST
|
||||
+ // Track remote addresses against local addresses (configured)
|
||||
+ // through `adb reverse` commands.
|
||||
+ // Access constrained to primary thread by virtue of check_main_thread().
|
||||
+ std::unordered_map<std::string, std::string> reverse_forwards_;
|
||||
+#endif
|
||||
+
|
||||
DISALLOW_COPY_AND_ASSIGN(atransport);
|
||||
};
|
||||
|
||||
--
|
||||
2.38.0
|
||||
|
|
@ -3,11 +3,12 @@ TERMUX_PKG_DESCRIPTION="Android platform tools"
|
|||
TERMUX_PKG_LICENSE="Apache-2.0, MIT"
|
||||
TERMUX_PKG_MAINTAINER="@termux"
|
||||
TERMUX_PKG_VERSION=31.0.3p1
|
||||
TERMUX_PKG_REVISION=6
|
||||
TERMUX_PKG_REVISION=7
|
||||
TERMUX_PKG_SRCURL=https://github.com/nmeum/android-tools/releases/download/$TERMUX_PKG_VERSION/android-tools-$TERMUX_PKG_VERSION.tar.xz
|
||||
TERMUX_PKG_SHA256=0ef69f919d58a2bdff2083d2e83a9ef38df079ec82651b2544e9e48086df5ab8
|
||||
TERMUX_PKG_AUTO_UPDATE=true
|
||||
TERMUX_PKG_DEPENDS="libc++, libusb, pcre2, googletest, libprotobuf, brotli, zstd, liblz4"
|
||||
TERMUX_PKG_DEPENDS="brotli, libc++, liblz4, libprotobuf, libusb, zlib, zstd"
|
||||
TERMUX_PKG_BUILD_DEPENDS="googletest, pcre2"
|
||||
|
||||
termux_step_pre_configure() {
|
||||
termux_setup_protobuf
|
||||
|
|
Loading…
Reference in New Issue