Merge pull request #10808

b305c44 escape control characters in json string serialiser (alhudz)

ACKs: vtnerd*, selsta

contrib/epee/src/parserse_base_utils.cpp

@@ -41,8 +41,14 @@ namespace misc_utils
   {
     std::string transform_to_escape_sequence(const std::string& src)
     {
-      static const char escaped[] = "\b\f\n\r\t\v\"\\/";
-      std::string::const_iterator it = std::find_first_of(src.begin(), src.end(), escaped, escaped + sizeof(escaped));
+      // RFC 8259: the double quote, reverse solidus and every control character
+      // (U+0000 - U+001F) must be escaped inside a JSON string. The forward slash
+      // is escaped too, as it has been historically.
+      std::string::const_iterator it = std::find_if(src.begin(), src.end(),
+        [](char ch) {
+          const unsigned char c = static_cast<unsigned char>(ch);
+          return c < 0x20 || c == '"' || c == '\\' || c == '/';
+        });
       if (it == src.end())
         return src;
 
@@ -51,7 +57,8 @@ namespace misc_utils
       res.assign(src.begin(), it);
       for(; it!=src.end(); ++it)
       {
-        switch(*it)
+        const unsigned char c = static_cast<unsigned char>(*it);
+        switch(c)
         {
         case '\b':  //Backspace (ascii code 08)
           res+="\\b"; break;
@@ -63,18 +70,22 @@ namespace misc_utils
           res+="\\r"; break;
         case '\t':  //Tab
           res+="\\t"; break;
-        case '\v':  //Vertical tab
-          res+="\\v"; break;
-        //case '\'':  //Apostrophe or single quote
-        //  res+="\\'"; break;
         case '"':  //Double quote
           res+="\\\""; break;
         case '\\':  //Backslash character
           res+="\\\\"; break;
-        case '/':  //Backslash character
+        case '/':  //Slash character
           res+="\\/"; break;
         default:
-          res.push_back(*it);
+          if (c < 0x20)
+          {
+            //control character without a short escape (\v is not valid JSON)
+            static const char hex[] = "0123456789abcdef";
+            const char u[] = { '\\', 'u', '0', '0', hex[c >> 4], hex[c & 0xf] };
+            res.append(u, sizeof(u));
+          }
+          else
+            res.push_back(*it);
         }
       }
       return res;

tests/unit_tests/epee_serialization.cpp

@@ -32,6 +32,7 @@
 #include <vector>
 
 #include "serialization/keyvalue_serialization.h"
+#include "storages/parserse_base_utils.h"
 #include "storages/portable_storage.h"
 #include "storages/portable_storage_template_helper.h"
 #include "span.h"
@@ -107,6 +108,15 @@ struct ObjWithBool
     KV_SERIALIZE(b)
   END_KV_SERIALIZE_MAP()
 };
+
+struct ObjWithString
+{
+  std::string s;
+
+  BEGIN_KV_SERIALIZE_MAP()
+    KV_SERIALIZE(s)
+  END_KV_SERIALIZE_MAP()
+};
 }
 
 TEST(epee_json, keyword_values)
@@ -129,6 +139,36 @@ TEST(epee_json, keyword_values)
   EXPECT_FALSE(epee::serialization::load_t_from_json(o, std::string("{\"b\": \xc3\x28}")));
 }
 
+TEST(epee_json, escape_control_characters)
+{
+  using epee::misc_utils::parse::transform_to_escape_sequence;
+
+  // control characters must be escaped (RFC 8259): the short escapes are used
+  // where JSON defines them and \u00xx otherwise. \v is not a valid JSON escape.
+  EXPECT_EQ(transform_to_escape_sequence(std::string("\x01")), "\\u0001");
+  EXPECT_EQ(transform_to_escape_sequence(std::string("\x07")), "\\u0007");
+  EXPECT_EQ(transform_to_escape_sequence(std::string("\x0b")), "\\u000b");
+  EXPECT_EQ(transform_to_escape_sequence(std::string("\x1f")), "\\u001f");
+  EXPECT_EQ(transform_to_escape_sequence(std::string("a\x00" "b", 3)), "a\\u0000b");
+
+  // short escapes and printable text are unchanged
+  EXPECT_EQ(transform_to_escape_sequence(std::string("a\tb\n\r\f\b")), "a\\tb\\n\\r\\f\\b");
+  EXPECT_EQ(transform_to_escape_sequence(std::string("plain text")), "plain text");
+
+  // control characters round trip through the json serialiser and parser, and
+  // never appear verbatim in the serialised output
+  ObjWithString o{};
+  o.s = std::string("x\x01\x0b" "y", 4);
+  std::string j;
+  EXPECT_TRUE(epee::serialization::store_t_to_json(o, j));
+  EXPECT_EQ(j.find('\x01'), std::string::npos);
+  EXPECT_EQ(j.find('\x0b'), std::string::npos);
+
+  ObjWithString o2{};
+  EXPECT_TRUE(epee::serialization::load_t_from_json(o2, j));
+  EXPECT_EQ(o2.s, o.s);
+}
+
 TEST(epee_binary, serialize_deserialize)
 {
   ParentObjWithOptChild<ObjWithOptChild> o;