From 32417fb37642a1f975ec107e6b449ba71196db56 Mon Sep 17 00:00:00 2001
From: Nick Kralevich <nnk@google.com>
Date: Wed, 23 Jan 2013 09:28:35 -0800
Subject: [PATCH] libc: use more secure system properties if available

Currently, system properties are passed via the environment
variable ANDROID_PROPERTY_WORKSPACE and a file descriptor passed
from parent to child. This is insecure for setuid executables,
as the environment variable can be changed by the caller.

Modify system property handling so that we get the properties
from a root owned properties file, rather than using an
environment variable.  Fall back to the environment variable
if the file doesn't exist.

Bug: 8045561
Change-Id: I54f3efa98cf7d63d88788da5ce0d19e34fd7851a
---
 libc/bionic/system_properties.c       | 61 +++++++++++++++++++--------
 libc/include/sys/_system_properties.h |  1 +
 2 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/libc/bionic/system_properties.c b/libc/bionic/system_properties.c
index caa5ca61b..a1312af11 100644
--- a/libc/bionic/system_properties.c
+++ b/libc/bionic/system_properties.c
@@ -31,12 +31,15 @@
 #include <stddef.h>
 #include <errno.h>
 #include <poll.h>
+#include <fcntl.h>
+#include <stdbool.h>
 
 #include <sys/mman.h>
 
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/select.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <netinet/in.h>
 #include <unistd.h>
@@ -52,36 +55,62 @@ static unsigned dummy_props = 0;
 
 prop_area *__system_property_area__ = (void*) &dummy_props;
 
+static int get_fd_from_env(void)
+{
+    char *env = getenv("ANDROID_PROPERTY_WORKSPACE");
+
+    if (!env) {
+        return -1;
+    }
+
+    return atoi(env);
+}
+
 int __system_properties_init(void)
 {
-    prop_area *pa;
-    int s, fd;
-    unsigned sz;
-    char *env;
+    bool fromFile = true;
 
     if(__system_property_area__ != ((void*) &dummy_props)) {
         return 0;
     }
 
-    env = getenv("ANDROID_PROPERTY_WORKSPACE");
-    if (!env) {
+    int fd = open(PROP_FILENAME, O_RDONLY | O_NOFOLLOW);
+
+    if ((fd < 0) && (errno == ENOENT)) {
+        /*
+         * For backwards compatibility, if the file doesn't
+         * exist, we use the environment to get the file descriptor.
+         * For security reasons, we only use this backup if the kernel
+         * returns ENOENT. We don't want to use the backup if the kernel
+         * returns other errors such as ENOMEM or ENFILE, since it
+         * might be possible for an external program to trigger this
+         * condition.
+         */
+        fd = get_fd_from_env();
+        fromFile = false;
+    }
+
+    if (fd < 0) {
         return -1;
     }
-    fd = atoi(env);
-    env = strchr(env, ',');
-    if (!env) {
+
+    struct stat fd_stat;
+    if (fstat(fd, &fd_stat) < 0) {
         return -1;
     }
-    sz = atoi(env + 1);
-    
-    pa = mmap(0, sz, PROT_READ, MAP_SHARED, fd, 0);
-    
-    if(pa == MAP_FAILED) {
+
+    prop_area *pa = mmap(0, fd_stat.st_size, PROT_READ, MAP_SHARED, fd, 0);
+
+    if (fromFile) {
+        close(fd);
+    }
+
+    if (pa == MAP_FAILED) {
         return -1;
     }
 
     if((pa->magic != PROP_AREA_MAGIC) || (pa->version != PROP_AREA_VERSION)) {
-        munmap(pa, sz);
+        munmap(pa, fd_stat.st_size);
         return -1;
     }
 
@@ -218,8 +247,6 @@ static int send_prop_msg(prop_msg *msg)
 int __system_property_set(const char *key, const char *value)
 {
     int err;
-    int tries = 0;
-    int update_seen = 0;
     prop_msg msg;
 
     if(key == 0) return -1;
diff --git a/libc/include/sys/_system_properties.h b/libc/include/sys/_system_properties.h
index 42a7f6c0f..10c0fae16 100644
--- a/libc/include/sys/_system_properties.h
+++ b/libc/include/sys/_system_properties.h
@@ -41,6 +41,7 @@ typedef struct prop_msg prop_msg;
 #define PROP_AREA_VERSION 0x45434f76
 
 #define PROP_SERVICE_NAME "property_service"
+#define PROP_FILENAME "/dev/__properties__"
 
 /* #define PROP_MAX_ENTRIES 247 */
 /* 247 -> 32620 bytes (<32768) */