Fix a unicode parsing error (#4310)

osquery · Apr 25, 2018 · c646139 · c646139
1 parent 8c99000
commit c646139
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 2 deletions.
diff --git a/osquery/core/conversions.cpp b/osquery/core/conversions.cpp
@@ -231,8 +231,13 @@ Status JSON::toString(std::string& str) const {
 }
 
 Status JSON::fromString(const std::string& str) {
-  if (doc_.Parse(str.c_str()).HasParseError()) {
-    return Status(1, "Cannot parse JSON");
+  rj::ParseResult pr = doc_.Parse(str.c_str());
+  if (!pr) {
+    std::string message{"Cannot parse JSON: "};
+    message += GetParseError_En(pr.Code());
+    message += " Offset: ";
+    message += std::to_string(pr.Offset());
+    return Status(1, message);
   }
   return Status();
 }

diff --git a/osquery/core/conversions.h b/osquery/core/conversions.h
@@ -20,6 +20,7 @@
 #include <boost/bind.hpp>
 #include <boost/shared_ptr.hpp>
 
+#include <osquery/logger.h>
 #include <osquery/status.h>
 
 #ifdef DARWIN
@@ -208,13 +209,22 @@ inline std::string unescapeUnicode(const std::string& escaped) {
       long value{0};
       Status stat = safeStrtol(escaped.substr(i + 2, 4), 16, value);
       if (!stat.ok()) {
+        LOG(WARNING) << "Unescaping a string with length: " << escaped.size()
+                     << " failed at: " << i;
         return "";
       }
       if (value < 255) {
         unescaped += static_cast<char>(value);
         i += 5;
         continue;
       }
+    } else if (i < escaped.size() - 1 && '\\' == escaped[i] &&
+               '\\' == escaped[i + 1]) {
+      // In the case of \\users 'sers' is not a unicode character
+      // If we see \\ we should skip them and we do this by adding
+      // an extra jump forward.
+      unescaped += escaped[i];
+      ++i;
     }
     unescaped += escaped[i];
   }

diff --git a/osquery/core/tests/conversions_tests.cpp b/osquery/core/tests/conversions_tests.cpp
@@ -67,6 +67,10 @@ TEST_F(ConversionsTests, test_unicode_unescape) {
       std::make_pair("\\uFFFFhi", "\\uFFFFhi"),
       std::make_pair("0000\\u", "0000\\u"),
       std::make_pair("hi", "hi"),
+      std::make_pair("c:\\\\users\\\\obelisk\\\\file.txt",
+                     "c:\\\\users\\\\obelisk\\\\file.txt"),
+      std::make_pair("Edge case test\\", "Edge case test\\"),
+      std::make_pair("Edge case test two\\\\", "Edge case test two\\\\"),
   };
 
   for (const auto& test : conversions) {
@@ -152,6 +156,14 @@ TEST_F(ConversionsTests, test_json_from_string) {
   EXPECT_FALSE(doc.fromString(json).ok());
 }
 
+TEST_F(ConversionsTests, test_json_from_string_error) {
+  std::string json = "{\"key\":\"value\",\"key2\":{\"key3\":'error'}}";
+  auto doc = JSON::newObject();
+  auto s = doc.fromString(json);
+  EXPECT_FALSE(s.ok());
+  EXPECT_EQ(s.getMessage(), "Cannot parse JSON: Invalid value. Offset: 30");
+}
+
 TEST_F(ConversionsTests, test_json_add_object) {
   std::string json = "{\"key\":\"value\", \"key2\":{\"key3\":[3,2,1]}}";
   auto doc = JSON::newObject();