From 87b3402b1f9265f51193ef1ae8c3c9082aa3e980 Mon Sep 17 00:00:00 2001
From: Mitchell Grenier <obelisk@fb.com>
Date: Sat, 21 Apr 2018 11:47:34 -0700
Subject: [PATCH] Fix a unicode parsing error

---
 osquery/core/conversions.cpp             | 11 ++++++++---
 osquery/core/conversions.h               | 10 ++++++++++
 osquery/core/tests/conversions_tests.cpp |  4 ++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/osquery/core/conversions.cpp b/osquery/core/conversions.cpp
index 6068485ffca..5a44aacbe92 100644
--- a/osquery/core/conversions.cpp
+++ b/osquery/core/conversions.cpp
@@ -231,10 +231,15 @@ 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 += pr.Offset();
+    return Status(1, message);
   }
-  return Status();
+  return Status{0};
 }
 
 void JSON::mergeObject(rj::Value& target_obj, rj::Value& source_obj) {
diff --git a/osquery/core/conversions.h b/osquery/core/conversions.h
index fe2f5c09157..8200120eb78 100644
--- 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,6 +209,8 @@ 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) {
@@ -215,6 +218,13 @@ inline std::string unescapeUnicode(const std::string& escaped) {
         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
index 100c87d9e2a..3cbe7a5ccae 100644
--- 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) {