libsysutils: Fix NetLinkEvent security issues.
The issues were the following:
- The code in decode() didn't handle the degenerate case where the input buffer is full of '@'
- The code in decode() assumed the input buffer is properly zero-terminated.
- The code in decode() would not check that it doesn't overwrite the mParams[] array.
- The code in findParam() would check mParams[i] before checking the value of 'i'
Also remove un-necessary calls to strlen() at runtime.
Change-Id: I8acead959bd10d97c5380b08958fcb796248a010
diff --git a/libsysutils/src/NetlinkEvent.cpp b/libsysutils/src/NetlinkEvent.cpp
index 86c1f42..c8d3b1f 100644
--- a/libsysutils/src/NetlinkEvent.cpp
+++ b/libsysutils/src/NetlinkEvent.cpp
@@ -56,45 +56,76 @@
}
}
+/* If the string between 'str' and 'end' begins with 'prefixlen' characters
+ * from the 'prefix' array, then return 'str + prefixlen', otherwise return
+ * NULL.
+ */
+static const char*
+has_prefix(const char* str, const char* end, const char* prefix, size_t prefixlen)
+{
+ if ((end-str) >= (ptrdiff_t)prefixlen && !memcmp(str, prefix, prefixlen))
+ return str + prefixlen;
+ else
+ return NULL;
+}
+
+/* Same as strlen(x) for constant string literals ONLY */
+#define CONST_STRLEN(x) (sizeof(x)-1)
+
+/* Convenience macro to call has_prefix with a constant string literal */
+#define HAS_CONST_PREFIX(str,end,prefix) has_prefix((str),(end),prefix,CONST_STRLEN(prefix))
+
+
bool NetlinkEvent::decode(char *buffer, int size) {
- char *s = buffer;
- char *end;
+ const char *s = buffer;
+ const char *end;
int param_idx = 0;
int i;
int first = 1;
+ if (size == 0)
+ return false;
+
+ /* Ensure the buffer is zero-terminated, the code below depends on this */
+ buffer[size-1] = '\0';
+
end = s + size;
while (s < end) {
if (first) {
- char *p;
- for (p = s; *p != '@'; p++);
- p++;
- mPath = strdup(p);
+ const char *p;
+ /* buffer is 0-terminated, no need to check p < end */
+ for (p = s; *p != '@'; p++) {
+ if (!*p) { /* no '@', should not happen */
+ return false;
+ }
+ }
+ mPath = strdup(p+1);
first = 0;
} else {
- if (!strncmp(s, "ACTION=", strlen("ACTION="))) {
- char *a = s + strlen("ACTION=");
+ const char* a;
+ if ((a = HAS_CONST_PREFIX(s, end, "ACTION=")) != NULL) {
if (!strcmp(a, "add"))
mAction = NlActionAdd;
else if (!strcmp(a, "remove"))
mAction = NlActionRemove;
else if (!strcmp(a, "change"))
mAction = NlActionChange;
- } else if (!strncmp(s, "SEQNUM=", strlen("SEQNUM=")))
- mSeq = atoi(s + strlen("SEQNUM="));
- else if (!strncmp(s, "SUBSYSTEM=", strlen("SUBSYSTEM=")))
- mSubsystem = strdup(s + strlen("SUBSYSTEM="));
- else
+ } else if ((a = HAS_CONST_PREFIX(s, end, "SEQNUM=")) != NULL) {
+ mSeq = atoi(a);
+ } else if ((a = HAS_CONST_PREFIX(s, end, "SUBSYSTEM=")) != NULL) {
+ mSubsystem = strdup(a);
+ } else if (param_idx < NL_PARAMS_MAX) {
mParams[param_idx++] = strdup(s);
+ }
}
- s+= strlen(s) + 1;
+ s += strlen(s) + 1;
}
return true;
}
const char *NetlinkEvent::findParam(const char *paramName) {
size_t len = strlen(paramName);
- for (int i = 0; mParams[i] && i < NL_PARAMS_MAX; ++i) {
+ for (int i = 0; i < NL_PARAMS_MAX && mParams[i] != NULL; ++i) {
const char *ptr = mParams[i] + len;
if (!strncmp(mParams[i], paramName, len) && *ptr == '=')
return ++ptr;