summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSiarhei Vishniakou <svv@google.com>2018-11-16 22:18:53 -0800
committerJP Sugarbroad <jpsugar@google.com>2019-01-14 14:54:51 -0800
commit00f21b8429b8b081c33b89c68577b6dcf78324c2 (patch)
tree83aa7ad5b061c96a5c1b3aa82b57b3f9b1c6469d
parent95993aca64af4dae59c87cc531e88158c9b72a80 (diff)
downloadnative-nougat-mr1.8-release.tar.gz
Sanitize InputMessage before sendingnougat-mr1.8-release
The struct InputMessage has many fields, and is force-aligned to 8-byte boundaries. There are also some padding fields that carry no information. This struct is typically allocated in the stack and populated with various values before being sent across as a stream of bytes through the socket. Therefore, the "unused" data portions of the struct could contain portions of the stack, since there aren't ever writes to those memory locations. To avoid this information leak, forcefully sanitize the struct. Create a new struct that is explicitly set to zero. Next, only fill the meaningful fields manually. Bug: 115739809 Test: cts-tradefed run cts -m CtsSecurityBulletinHostTestCases -t android.security.cts.Poc18_12; adb shell monkey 100000 Change-Id: I7e44dacf1e8fa3156c8e4d2f7784ef0c53dab507 Merged-In: I7e44dacf1e8fa3156c8e4d2f7784ef0c53dab507 (cherry picked from commit cb2f0ceed876b3c1cc62fc677c75a12c1f10b199)
-rw-r--r--include/input/InputTransport.h11
-rw-r--r--libs/input/InputTransport.cpp102
-rw-r--r--libs/input/tests/StructLayout_test.cpp3
3 files changed, 114 insertions, 2 deletions
diff --git a/include/input/InputTransport.h b/include/input/InputTransport.h
index f31bceabd7..7d56bf5b46 100644
--- a/include/input/InputTransport.h
+++ b/include/input/InputTransport.h
@@ -42,6 +42,13 @@ namespace android {
*
* Note that this structure is used for IPCs so its layout must be identical
* on 64 and 32 bit processes. This is tested in StructLayout_test.cpp.
+ *
+ * Since the struct must be aligned to an 8-byte boundary, there could be uninitialized bytes
+ * in-between the defined fields. This padding data should be explicitly accounted for by adding
+ * "empty" fields into the struct. This data is memset to zero before sending the struct across
+ * the socket. Adding the explicit fields ensures that the memset is not optimized away by the
+ * compiler. When a new field is added to the struct, the corresponding change
+ * in StructLayout_test should be made.
*/
struct InputMessage {
enum {
@@ -62,6 +69,7 @@ struct InputMessage {
union Body {
struct Key {
uint32_t seq;
+ uint32_t empty1;
nsecs_t eventTime __attribute__((aligned(8)));
int32_t deviceId;
int32_t source;
@@ -80,6 +88,7 @@ struct InputMessage {
struct Motion {
uint32_t seq;
+ uint32_t empty1;
nsecs_t eventTime __attribute__((aligned(8)));
int32_t deviceId;
int32_t source;
@@ -95,6 +104,7 @@ struct InputMessage {
float xPrecision;
float yPrecision;
uint32_t pointerCount;
+ uint32_t empty2;
// Note that PointerCoords requires 8 byte alignment.
struct Pointer {
PointerProperties properties;
@@ -125,6 +135,7 @@ struct InputMessage {
bool isValid(size_t actualSize) const;
size_t size() const;
+ void getSanitizedCopy(InputMessage* msg) const;
};
/*
diff --git a/libs/input/InputTransport.cpp b/libs/input/InputTransport.cpp
index 2dff4e0918..f6d2c15633 100644
--- a/libs/input/InputTransport.cpp
+++ b/libs/input/InputTransport.cpp
@@ -97,6 +97,102 @@ size_t InputMessage::size() const {
return sizeof(Header);
}
+/**
+ * There could be non-zero bytes in-between InputMessage fields. Force-initialize the entire
+ * memory to zero, then only copy the valid bytes on a per-field basis.
+ */
+void InputMessage::getSanitizedCopy(InputMessage* msg) const {
+ memset(msg, 0, sizeof(*msg));
+
+ // Write the header
+ msg->header.type = header.type;
+
+ // Write the body
+ switch(header.type) {
+ case InputMessage::TYPE_KEY: {
+ // uint32_t seq
+ msg->body.key.seq = body.key.seq;
+ // nsecs_t eventTime
+ msg->body.key.eventTime = body.key.eventTime;
+ // int32_t deviceId
+ msg->body.key.deviceId = body.key.deviceId;
+ // int32_t source
+ msg->body.key.source = body.key.source;
+ // int32_t action
+ msg->body.key.action = body.key.action;
+ // int32_t flags
+ msg->body.key.flags = body.key.flags;
+ // int32_t keyCode
+ msg->body.key.keyCode = body.key.keyCode;
+ // int32_t scanCode
+ msg->body.key.scanCode = body.key.scanCode;
+ // int32_t metaState
+ msg->body.key.metaState = body.key.metaState;
+ // int32_t repeatCount
+ msg->body.key.repeatCount = body.key.repeatCount;
+ // nsecs_t downTime
+ msg->body.key.downTime = body.key.downTime;
+ break;
+ }
+ case InputMessage::TYPE_MOTION: {
+ // uint32_t seq
+ msg->body.motion.seq = body.motion.seq;
+ // nsecs_t eventTime
+ msg->body.motion.eventTime = body.motion.eventTime;
+ // int32_t deviceId
+ msg->body.motion.deviceId = body.motion.deviceId;
+ // int32_t source
+ msg->body.motion.source = body.motion.source;
+ // int32_t action
+ msg->body.motion.action = body.motion.action;
+ // int32_t actionButton
+ msg->body.motion.actionButton = body.motion.actionButton;
+ // int32_t flags
+ msg->body.motion.flags = body.motion.flags;
+ // int32_t metaState
+ msg->body.motion.metaState = body.motion.metaState;
+ // int32_t buttonState
+ msg->body.motion.buttonState = body.motion.buttonState;
+ // int32_t edgeFlags
+ msg->body.motion.edgeFlags = body.motion.edgeFlags;
+ // nsecs_t downTime
+ msg->body.motion.downTime = body.motion.downTime;
+ // float xOffset
+ msg->body.motion.xOffset = body.motion.xOffset;
+ // float yOffset
+ msg->body.motion.yOffset = body.motion.yOffset;
+ // float xPrecision
+ msg->body.motion.xPrecision = body.motion.xPrecision;
+ // float yPrecision
+ msg->body.motion.yPrecision = body.motion.yPrecision;
+ // uint32_t pointerCount
+ msg->body.motion.pointerCount = body.motion.pointerCount;
+ //struct Pointer pointers[MAX_POINTERS]
+ for (size_t i = 0; i < body.motion.pointerCount; i++) {
+ // PointerProperties properties
+ msg->body.motion.pointers[i].properties.id = body.motion.pointers[i].properties.id;
+ msg->body.motion.pointers[i].properties.toolType =
+ body.motion.pointers[i].properties.toolType,
+ // PointerCoords coords
+ msg->body.motion.pointers[i].coords.bits = body.motion.pointers[i].coords.bits;
+ const uint32_t count = BitSet64::count(body.motion.pointers[i].coords.bits);
+ memcpy(&msg->body.motion.pointers[i].coords.values[0],
+ &body.motion.pointers[i].coords.values[0],
+ count * (sizeof(body.motion.pointers[i].coords.values[0])));
+ }
+ break;
+ }
+ case InputMessage::TYPE_FINISHED: {
+ msg->body.finished.seq = body.finished.seq;
+ msg->body.finished.handled = body.finished.handled;
+ break;
+ }
+ default: {
+ LOG_FATAL("Unexpected message type %i", header.type);
+ break;
+ }
+ }
+}
// --- InputChannel ---
@@ -150,10 +246,12 @@ status_t InputChannel::openInputChannelPair(const String8& name,
}
status_t InputChannel::sendMessage(const InputMessage* msg) {
- size_t msgLength = msg->size();
+ const size_t msgLength = msg->size();
+ InputMessage cleanMsg;
+ msg->getSanitizedCopy(&cleanMsg);
ssize_t nWrite;
do {
- nWrite = ::send(mFd, msg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL);
+ nWrite = ::send(mFd, &cleanMsg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL);
} while (nWrite == -1 && errno == EINTR);
if (nWrite < 0) {
diff --git a/libs/input/tests/StructLayout_test.cpp b/libs/input/tests/StructLayout_test.cpp
index 8d73f453e0..5172681e3b 100644
--- a/libs/input/tests/StructLayout_test.cpp
+++ b/libs/input/tests/StructLayout_test.cpp
@@ -63,6 +63,9 @@ void TestInputMessageAlignment() {
CHECK_OFFSET(InputMessage::Body::Motion, yPrecision, 68);
CHECK_OFFSET(InputMessage::Body::Motion, pointerCount, 72);
CHECK_OFFSET(InputMessage::Body::Motion, pointers, 80);
+
+ CHECK_OFFSET(InputMessage::Body::Finished, seq, 0);
+ CHECK_OFFSET(InputMessage::Body::Finished, handled, 4);
}
} // namespace android