commit 69261cb65186e27dfbdc1e3eec796437f9968ff9
author JP Abgrall <jpa@google.com> Thu Jun 19 18:35:24 2014 -0700
committer JP Abgrall <jpa@google.com> Thu Jun 19 19:14:32 2014 -0700
tree bf1b5f8cd7ccec519c8c9c6d33f5b61e4fcbb0e9
parent a561e121c724e9163b2e256e15eef660e3a326da

server: check interface names in RPC arguments for validity

This patch introduces a method isIfaceName that checks interface
names from various RPCs for validity before e.g. using them as
part of iptables arguments or in filenames.

All of these RPC calls can only be called from applications
with at least the CONNECTIVITY_INTERNAL permission in recent
Android versions, so the impact of the missing checks luckily
isn't very high.

Orig-Author: Jann Horn <jann@thejh.net>

Change-Id: I80df8d745a3de99ad02d6649f0d10562c81f6b98
Signed-off-by: JP Abgrall <jpa@google.com>

server/BandwidthController.cpp

diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 4f9344c..e1db680 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -607,6 +607,8 @@
         ALOGE("Invalid bytes value. 1..max_int64.");
         return -1;
     }
+    if (!isIfaceName(iface))
+        return -1;
     if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
         ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
         return -1;
@@ -667,6 +669,8 @@
     std::list<std::string>::iterator it;
     const char *costName = "shared";
 
+    if (!isIfaceName(iface))
+        return -1;
     if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
         ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
         return -1;
@@ -706,10 +710,8 @@
     std::list<QuotaInfo>::iterator it;
     std::string quotaCmd;
 
-    if (!isNameLegal(iface)) {
-        ALOGE("setInterfaceQuota: Invalid iface \"%s\"", iface);
+    if (!isIfaceName(iface))
         return -1;
-    }
 
     if (!maxBytes) {
         /* Don't talk about -1, deprecate it. */
@@ -775,36 +777,13 @@
     return getInterfaceQuota("shared", bytes);
 }
 
-bool BandwidthController::isNameLegal(const char* name) {
-    size_t i;
-    size_t name_len = strlen(name);
-    if (name_len == 0) {
-        return false;
-    }
-
-    /* First character must be alphanumeric */
-    if (!isalnum(name[0])) {
-        return false;
-    }
-
-    for (i = 1; i < name_len; i++) {
-        if (!isalnum(name[i]) && (name[i] != '_') && (name[i] != '-') && (name[i] != ':')) {
-            return false;
-        }
-    }
-
-    return true;
-}
-
 int BandwidthController::getInterfaceQuota(const char *costName, int64_t *bytes) {
     FILE *fp;
     char *fname;
     int scanRes;
 
-    if (!isNameLegal(costName)) {
-        ALOGE("getInterfaceQuota: Invalid costName \"%s\"", costName);
+    if (!isIfaceName(costName))
         return -1;
-    }
 
     asprintf(&fname, "/proc/net/xt_quota/%s", costName);
     fp = fopen(fname, "r");
@@ -827,11 +806,8 @@
     const char *costName;
     std::list<QuotaInfo>::iterator it;
 
-    if (!isNameLegal(iface)) {
-        ALOGE("removeInterfaceQuota: Invalid iface \"%s\"", iface);
+    if (!isIfaceName(iface))
         return -1;
-    }
-
     if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
         ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
         return -1;
@@ -861,7 +837,7 @@
     FILE *fp;
     char *fname;
 
-    if (!isNameLegal(quotaName)) {
+    if (!isIfaceName(quotaName)) {
         ALOGE("updateQuota: Invalid quotaName \"%s\"", quotaName);
         return -1;
     }
@@ -1040,7 +1016,7 @@
 int BandwidthController::setInterfaceAlert(const char *iface, int64_t bytes) {
     std::list<QuotaInfo>::iterator it;
 
-    if (!isNameLegal(iface)) {
+    if (!isIfaceName(iface)) {
         ALOGE("setInterfaceAlert: Invalid iface \"%s\"", iface);
         return -1;
     }
@@ -1065,7 +1041,7 @@
 int BandwidthController::removeInterfaceAlert(const char *iface) {
     std::list<QuotaInfo>::iterator it;
 
-    if (!isNameLegal(iface)) {
+    if (!isIfaceName(iface)) {
         ALOGE("removeInterfaceAlert: Invalid iface \"%s\"", iface);
         return -1;
     }
@@ -1089,7 +1065,7 @@
     int res = 0;
     char *alertName;
 
-    if (!isNameLegal(costName)) {
+    if (!isIfaceName(costName)) {
         ALOGE("setCostlyAlert: Invalid costName \"%s\"", costName);
         return -1;
     }
@@ -1119,7 +1095,7 @@
     char *alertName;
     int res = 0;
 
-    if (!isNameLegal(costName)) {
+    if (!isIfaceName(costName)) {
         ALOGE("removeCostlyAlert: Invalid costName \"%s\"", costName);
         return -1;
     }