SERVER-36272 Catch error in parsing values after % in URIs

10 files changed, 50 insertions, 19 deletions

diff --git a/src/mongo/bson/json.cpp b/src/mongo/bson/json.cpp
index f5909e6c3ff..3ad943fdac3 100644
--- a/src/mongo/bson/json.cpp
+++ b/src/mongo/bson/json.cpp
@@ -400,7 +400,8 @@ Status JParse::binaryObject(StringData fieldName, BSONObjBuilder& builder) {
     // unsigned char. If we don't coerce it to an unsigned char before
     // wrapping it in a BinDataType (currently implicitly a signed
     // integer), we get undefined behavior.
-    const auto binDataTypeNumeric = static_cast<unsigned char>(fromHex(binDataType));
+    const auto binDataTypeNumeric =
+        static_cast<unsigned char>(uassertStatusOK(fromHex(binDataType)));
 
     builder.appendBinData(
         fieldName, binData.length(), BinDataType(binDataTypeNumeric), binData.data());
@@ -1150,8 +1151,8 @@ Status JParse::chars(std::string* result, const char* terminalSet, const char* a
                     if (!isHexString(StringData(q, 4))) {
                         return parseError("Expecting 4 hex digits");
                     }
-                    unsigned char first = fromHex(q);
-                    unsigned char second = fromHex(q += 2);
+                    unsigned char first = uassertStatusOK(fromHex(q));
+                    unsigned char second = uassertStatusOK(fromHex(q += 2));
                     const std::string& utf8str = encodeUTF8(first, second);
                     for (unsigned int i = 0; i < utf8str.size(); i++) {
                         result->push_back(utf8str[i]);
diff --git a/src/mongo/bson/oid.cpp b/src/mongo/bson/oid.cpp
index 26d90d97bad..b875cdf10d6 100644
--- a/src/mongo/bson/oid.cpp
+++ b/src/mongo/bson/oid.cpp
@@ -151,7 +151,7 @@ void OID::init(const std::string& s) {
     verify(s.size() == 24);
     const char* p = s.c_str();
     for (std::size_t i = 0; i < kOIDSize; i++) {
-        _data[i] = fromHex(p);
+        _data[i] = uassertStatusOK(fromHex(p));
         p += 2;
     }
 }
diff --git a/src/mongo/client/mongo_uri.cpp b/src/mongo/client/mongo_uri.cpp
index ea455cb0794..6d4eb7f83b0 100644
--- a/src/mongo/client/mongo_uri.cpp
+++ b/src/mongo/client/mongo_uri.cpp
@@ -84,13 +84,20 @@ void mongo::uriEncode(std::ostream& ss, StringData toEncode, StringData passthro
 mongo::StatusWith<std::string> mongo::uriDecode(StringData toDecode) {
     StringBuilder out;
     for (size_t i = 0; i < toDecode.size(); ++i) {
-        const auto c = toDecode[i];
+        const char c = toDecode[i];
         if (c == '%') {
             if (i + 2 > toDecode.size()) {
                 return Status(ErrorCodes::FailedToParse,
                               "Encountered partial escape sequence at end of string");
             }
-            out << fromHex(toDecode.substr(i + 1, 2));
+            auto swHex = fromHex(toDecode.substr(i + 1, 2));
+            if (swHex.isOK()) {
+                out << swHex.getValue();
+            } else {
+                return Status(ErrorCodes::FailedToParse,
+                              "The characters after the % do not form a hex value. Please escape "
+                              "the % or pass a valid hex value. ");
+            }
             i += 2;
         } else {
             out << c;
diff --git a/src/mongo/client/mongo_uri_test.cpp b/src/mongo/client/mongo_uri_test.cpp
index 4d8088ea015..6e97d94e924 100644
--- a/src/mongo/client/mongo_uri_test.cpp
+++ b/src/mongo/client/mongo_uri_test.cpp
@@ -39,6 +39,8 @@
 #include "mongo/unittest/unittest.h"
 
 #include <boost/filesystem/operations.hpp>
+#include <boost/optional.hpp>
+#include <boost/optional/optional_io.hpp>
 
 namespace mongo {
 namespace {
@@ -55,6 +57,7 @@ struct URITestCase {
 
 struct InvalidURITestCase {
     std::string URI;
+    boost::optional<Status> status = boost::none;
 };
 
 const ConnectionString::ConnectionType kMaster = ConnectionString::MASTER;
@@ -354,6 +357,12 @@ const InvalidURITestCase invalidCases[] = {
     // Host list must actually be comma separated.
     {"mongodb://localhost:27017localhost:27018"},
 
+    // % symbol in password must be escaped.
+    {"mongodb://localhost:pass%word@127.0.0.1:27017",
+     Status(ErrorCodes::FailedToParse,
+            "The characters after the % do not form a hex value. Please escape the % or pass a "
+            "valid hex value. ")},
+
     // Domain sockets have to end in ".sock".
     {"mongodb://%2Fnotareal%2Fdomainsock"},
 
@@ -465,6 +474,9 @@ TEST(MongoURI, InvalidURIs) {
         unittest::log() << "Testing URI: " << testCase.URI << '\n';
         auto cs_status = MongoURI::parse(testCase.URI);
         ASSERT_NOT_OK(cs_status);
+        if (testCase.status) {
+            ASSERT_EQUALS(testCase.status, cs_status.getStatus());
+        }
     }
 }
 
diff --git a/src/mongo/db/storage/key_string_test.cpp b/src/mongo/db/storage/key_string_test.cpp
index 9a8e4d78a70..dd4561bcd16 100644
--- a/src/mongo/db/storage/key_string_test.cpp
+++ b/src/mongo/db/storage/key_string_test.cpp
@@ -280,7 +280,7 @@ TEST_F(KeyStringTest, ActualBytesDouble) {
     // last byte (kEnd) doesn't get flipped
     string hexFlipped;
     for (size_t i = 0; i < hex.size() - 2; i += 2) {
-        char c = fromHex(hex.c_str() + i);
+        char c = uassertStatusOK(fromHex(hex.c_str() + i));
         c = ~c;
         hexFlipped += toHex(&c, 1);
     }
diff --git a/src/mongo/platform/decimal128_bson_test.cpp b/src/mongo/platform/decimal128_bson_test.cpp
index b0f453aff42..182b62822d8 100644
--- a/src/mongo/platform/decimal128_bson_test.cpp
+++ b/src/mongo/platform/decimal128_bson_test.cpp
@@ -56,7 +56,7 @@ BSONObj convertHexStringToBsonObj(StringData hexString) {
     auto buffer = SharedBuffer::allocate(bufferSize);
 
     for (unsigned int i = 0; i < bufferSize; i++) {
-        buffer.get()[i] = fromHex(p);
+        buffer.get()[i] = uassertStatusOK(fromHex(p));
         p += 2;
     }
 
diff --git a/src/mongo/scripting/mozjs/bindata.cpp b/src/mongo/scripting/mozjs/bindata.cpp
index 6d91dea8008..52ea3a9c4a2 100644
--- a/src/mongo/scripting/mozjs/bindata.cpp
+++ b/src/mongo/scripting/mozjs/bindata.cpp
@@ -86,7 +86,7 @@ void hexToBinData(JSContext* cx,
         int src_index = i * 2;
         if (!std::isxdigit(src[src_index]) || !std::isxdigit(src[src_index + 1]))
             uasserted(ErrorCodes::BadValue, "Invalid hex character in string");
-        data[i] = fromHex(src + src_index);
+        data[i] = uassertStatusOK(fromHex(src + src_index));
     }
 
     std::string encoded = base64::encode(data.get(), len);
diff --git a/src/mongo/util/hex.h b/src/mongo/util/hex.h
index 550fecc4cd5..48b17df4395 100644
--- a/src/mongo/util/hex.h
+++ b/src/mongo/util/hex.h
@@ -35,24 +35,35 @@
 
 #include "mongo/base/string_data.h"
 #include "mongo/bson/util/builder.h"
+#include "mongo/util/mongoutils/str.h"
 
 namespace mongo {
 // can't use hex namespace because it conflicts with hex iostream function
-inline int fromHex(char c) {
+inline StatusWith<char> fromHex(char c) {
     if ('0' <= c && c <= '9')
         return c - '0';
     if ('a' <= c && c <= 'f')
         return c - 'a' + 10;
     if ('A' <= c && c <= 'F')
         return c - 'A' + 10;
-    verify(false);
-    return 0xff;
+    return Status(ErrorCodes::FailedToParse,
+                  str::stream() << "The character " << c << " failed to parse from hex.");
 }
-inline char fromHex(const char* c) {
-    return (char)((fromHex(c[0]) << 4) | fromHex(c[1]));
+inline StatusWith<char> fromHex(const char* c) {
+    if (fromHex(c[0]).isOK() && fromHex(c[1]).isOK()) {
+        return (char)((fromHex(c[0]).getValue() << 4) | fromHex(c[1]).getValue());
+    }
+    return Status(ErrorCodes::FailedToParse,
+                  str::stream() << "The character " << c[0] << c[1]
+                                << " failed to parse from hex.");
 }
-inline char fromHex(StringData c) {
-    return (char)((fromHex(c[0]) << 4) | fromHex(c[1]));
+inline StatusWith<char> fromHex(StringData c) {
+    if (fromHex(c[0]).isOK() && fromHex(c[1]).isOK()) {
+        return (char)((fromHex(c[0]).getValue() << 4) | fromHex(c[1]).getValue());
+    }
+    return Status(ErrorCodes::FailedToParse,
+                  str::stream() << "The character " << c[0] << c[1]
+                                << " failed to parse from hex.");
 }
 
 /**
@@ -63,7 +74,7 @@ inline void fromHexString(StringData hexString, BufBuilder* buf) {
     invariant(hexString.size() % 2 == 0);
     // Combine every pair of two characters into one byte.
     for (std::size_t i = 0; i < hexString.size(); i += 2) {
-        buf->appendChar(fromHex(StringData(&hexString.rawData()[i], 2)));
+        buf->appendChar(uassertStatusOK(fromHex(StringData(&hexString.rawData()[i], 2))));
     }
 }
 
diff --git a/src/mongo/util/net/ssl_options.cpp b/src/mongo/util/net/ssl_options.cpp
index 31752ce6dfe..72d308841c0 100644
--- a/src/mongo/util/net/ssl_options.cpp
+++ b/src/mongo/util/net/ssl_options.cpp
@@ -66,7 +66,7 @@ StatusWith<std::vector<uint8_t>> hexToVector(StringData hex) {
     int idx = -2;
     std::generate(ret.begin(), ret.end(), [&hex, &idx] {
         idx += 2;
-        return (fromHex(hex[idx]) << 4) | fromHex(hex[idx + 1]);
+        return (uassertStatusOK(fromHex(hex[idx])) << 4) | uassertStatusOK(fromHex(hex[idx + 1]));
     });
     return ret;
 }
diff --git a/src/mongo/util/uuid.cpp b/src/mongo/util/uuid.cpp
index 32f69ca7d33..519a2b015b9 100644
--- a/src/mongo/util/uuid.cpp
+++ b/src/mongo/util/uuid.cpp
@@ -75,7 +75,7 @@ StatusWith<UUID> UUID::parse(const std::string& s) {
         char high = s[j++];
         char low = s[j++];
 
-        uuid[i] = ((fromHex(high) << 4) | fromHex(low));
+        uuid[i] = ((uassertStatusOK(fromHex(high)) << 4) | uassertStatusOK(fromHex(low)));
     }
 
     return UUID{std::move(uuid)};