From f7c74694d75cfa602a7ab096bcb5668852857a24 Mon Sep 17 00:00:00 2001
From: Paul Crowley <paulcrowley@google.com>
Date: Thu, 25 Aug 2022 11:38:27 -0700
Subject: [PATCH] Validate persistent properties file

Before loading persistent properties, init now checks if there are any
invalid properties (not starting with "persist.").

Bug: 243723877
Test: atest persistent_properties
Change-Id: Ieb4ddce05916f193388af6b658e1904004ffa473
---
 init/persistent_properties.cpp      | 28 +++++++++++++++++++++-------
 init/persistent_properties_test.cpp | 23 +++++++++++++++++++++++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/init/persistent_properties.cpp b/init/persistent_properties.cpp
index 716f62ec9..d33a6b8e2 100644
--- a/init/persistent_properties.cpp
+++ b/init/persistent_properties.cpp
@@ -155,19 +155,33 @@ Result<std::string> ReadPersistentPropertyFile() {
     return *file_contents;
 }
 
+Result<PersistentProperties> ParsePersistentPropertyFile(const std::string& file_contents) {
+    PersistentProperties persistent_properties;
+    if (!persistent_properties.ParseFromString(file_contents)) {
+        return Error() << "Unable to parse persistent property file: Could not parse protobuf";
+    }
+    for (auto& prop : persistent_properties.properties()) {
+        if (!StartsWith(prop.name(), "persist.")) {
+            return Error() << "Unable to load persistent property file: property '" << prop.name()
+                           << "' doesn't start with 'persist.'";
+        }
+    }
+    return persistent_properties;
+}
+
 }  // namespace
 
 Result<PersistentProperties> LoadPersistentPropertyFile() {
     auto file_contents = ReadPersistentPropertyFile();
     if (!file_contents.ok()) return file_contents.error();
 
-    PersistentProperties persistent_properties;
-    if (persistent_properties.ParseFromString(*file_contents)) return persistent_properties;
-
-    // If the file cannot be parsed in either format, then we don't have any recovery
-    // mechanisms, so we delete it to allow for future writes to take place successfully.
-    unlink(persistent_property_filename.c_str());
-    return Error() << "Unable to parse persistent property file: Could not parse protobuf";
+    auto persistent_properties = ParsePersistentPropertyFile(*file_contents);
+    if (!persistent_properties.ok()) {
+        // If the file cannot be parsed in either format, then we don't have any recovery
+        // mechanisms, so we delete it to allow for future writes to take place successfully.
+        unlink(persistent_property_filename.c_str());
+    }
+    return persistent_properties;
 }
 
 Result<void> WritePersistentPropertyFile(const PersistentProperties& persistent_properties) {
diff --git a/init/persistent_properties_test.cpp b/init/persistent_properties_test.cpp
index 60cecde25..e5d26dbef 100644
--- a/init/persistent_properties_test.cpp
+++ b/init/persistent_properties_test.cpp
@@ -155,5 +155,28 @@ TEST(persistent_properties, UpdatePropertyBadParse) {
     EXPECT_FALSE(it == read_back_properties.properties().end());
 }
 
+TEST(persistent_properties, RejectNonPersistProperty) {
+    TemporaryFile tf;
+    ASSERT_TRUE(tf.fd != -1);
+    persistent_property_filename = tf.path;
+
+    WritePersistentProperty("notpersist.sys.locale", "pt-BR");
+
+    auto read_back_properties = LoadPersistentProperties();
+    EXPECT_EQ(read_back_properties.properties().size(), 0);
+
+    WritePersistentProperty("persist.sys.locale", "pt-BR");
+
+    read_back_properties = LoadPersistentProperties();
+    EXPECT_GT(read_back_properties.properties().size(), 0);
+
+    auto it = std::find_if(read_back_properties.properties().begin(),
+                           read_back_properties.properties().end(), [](const auto& entry) {
+                               return entry.name() == "persist.sys.locale" &&
+                                      entry.value() == "pt-BR";
+                           });
+    EXPECT_FALSE(it == read_back_properties.properties().end());
+}
+
 }  // namespace init
 }  // namespace android