From b93cedafe455b95a3c4e0387bce4cf294c615068 Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Tue, 11 Apr 2017 18:30:42 +1200
Subject: [PATCH] torcontrol: Handle escapes in Tor QuotedStrings

https://trac.torproject.org/projects/tor/ticket/14999 is tracking an encoding
bug with the Tor control protocol, where many of the QuotedString instances that
Tor outputs are in fact CStrings, but it is not documented which ones are which.

https://spec.torproject.org/control-spec section 2.1.1 provides a future-proofed
rule for handing QuotedStrings, which this commit implements.
---
 src/test/torcontrol_tests.cpp | 30 +++++++++++++++++-----
 src/torcontrol.cpp            | 48 ++++++++++++++++++++++++++++++++---
 2 files changed, 68 insertions(+), 10 deletions(-)

diff --git a/src/test/torcontrol_tests.cpp b/src/test/torcontrol_tests.cpp
index 68516599d..8d3bb8f77 100644
--- a/src/test/torcontrol_tests.cpp
+++ b/src/test/torcontrol_tests.cpp
@@ -119,27 +119,43 @@ BOOST_AUTO_TEST_CASE(util_ParseTorReplyMapping)
             {"Foo", "Bar Baz"},
         });
 
-    // Escapes (which are left escaped by the parser)
+    // Escapes
     CheckParseTorReplyMapping(
         "Foo=\"Bar\\ Baz\"", {
-            {"Foo", "Bar\\ Baz"},
+            {"Foo", "Bar Baz"},
         });
     CheckParseTorReplyMapping(
         "Foo=\"Bar\\Baz\"", {
-            {"Foo", "Bar\\Baz"},
+            {"Foo", "BarBaz"},
         });
     CheckParseTorReplyMapping(
         "Foo=\"Bar\\@Baz\"", {
-            {"Foo", "Bar\\@Baz"},
+            {"Foo", "Bar@Baz"},
         });
     CheckParseTorReplyMapping(
         "Foo=\"Bar\\\"Baz\" Spam=\"\\\"Eggs\\\"\"", {
-            {"Foo", "Bar\\\"Baz"},
-            {"Spam", "\\\"Eggs\\\""},
+            {"Foo", "Bar\"Baz"},
+            {"Spam", "\"Eggs\""},
         });
     CheckParseTorReplyMapping(
         "Foo=\"Bar\\\\Baz\"", {
-            {"Foo", "Bar\\\\Baz"},
+            {"Foo", "Bar\\Baz"},
+        });
+
+    // C escapes
+    CheckParseTorReplyMapping(
+        "Foo=\"Bar\\nBaz\\t\" Spam=\"\\rEggs\" Octals=\"\\1a\\11\\17\\18\\81\\377\\378\\400\" Final=Check", {
+            {"Foo", "Bar\nBaz\t"},
+            {"Spam", "\rEggs"},
+            {"Octals", "\1a\11\17\1" "881\377\37" "8400"},
+            {"Final", "Check"},
+        });
+    CheckParseTorReplyMapping(
+        "Valid=Mapping Bare=\"Escape\\\"", {});
+    CheckParseTorReplyMapping(
+        "OneOctal=\"OneEnd\\1\" TwoOctal=\"TwoEnd\\11\"", {
+            {"OneOctal", "OneEnd\1"},
+            {"TwoOctal", "TwoEnd\11"},
         });
 
     // A more complex valid grammar. PROTOCOLINFO accepts a VersionLine that
diff --git a/src/torcontrol.cpp b/src/torcontrol.cpp
index 60181b440..b2f229cba 100644
--- a/src/torcontrol.cpp
+++ b/src/torcontrol.cpp
@@ -292,10 +292,52 @@ static std::map<std::string,std::string> ParseTorReplyMapping(const std::string
             if (ptr == s.size()) // unexpected end of line
                 return std::map<std::string,std::string>();
             ++ptr; // skip closing '"'
-            /* TODO: unescape value - according to the spec this depends on the
-             * context, some strings use C-LogPrintf style escape codes, some
-             * don't. So may be better handled at the call site.
+            /**
+             * Escape value. Per https://spec.torproject.org/control-spec section 2.1.1:
+             *
+             *   For future-proofing, controller implementors MAY use the following
+             *   rules to be compatible with buggy Tor implementations and with
+             *   future ones that implement the spec as intended:
+             *
+             *     Read \n \t \r and \0 ... \377 as C escapes.
+             *     Treat a backslash followed by any other character as that character.
              */
+            std::string escaped_value;
+            for (size_t i = 0; i < value.size(); ++i) {
+                if (value[i] == '\\') {
+                    // This will always be valid, because if the final character
+                    // in the QuotedString was a \ then the parser would already
+                    // have returned above, due to a missing terminating
+                    // double-quote.
+                    ++i;
+                    if (value[i] == 'n') {
+                        escaped_value.push_back('\n');
+                    } else if (value[i] == 't') {
+                        escaped_value.push_back('\t');
+                    } else if (value[i] == 'r') {
+                        escaped_value.push_back('\r');
+                    } else if ('0' <= value[i] && value[i] <= '7') {
+                        size_t j;
+                        // Octal escape sequences have a limit of three octal digits,
+                        // but terminate at the first character that is not a valid
+                        // octal digit if encountered sooner.
+                        for (j = 1; '0' <= value[i+j] && value[i+j] <= '7' && j < 3; ++j) {}
+                        // Tor restricts first digit to 0-3 for three-digit octals.
+                        if (j < 3 || ('0' <= value[i] && value[i] <= '3')) {
+                            escaped_value.push_back(strtol(value.substr(i, j).c_str(), NULL, 8));
+                            // Account for automatic incrementing at loop end
+                            i += j - 1;
+                        } else {
+                            escaped_value.push_back(value[i]);
+                        }
+                    } else {
+                        escaped_value.push_back(value[i]);
+                    }
+                } else {
+                    escaped_value.push_back(value[i]);
+                }
+            }
+            value = escaped_value;
         } else { // Unquoted value. Note that values can contain '=' at will, just no spaces
             while (ptr < s.size() && s[ptr] != ' ') {
                 value.push_back(s[ptr]);