Improve validation of JSON inputs

src/ripple/types/api/strHex.h

@@ -27,8 +27,20 @@
 
 namespace ripple {
 
+/** Converts an integer to the corresponding hex digit 
+    @param iDigit 0-15 inclusive
+    @return a character from '0'-'9' or 'A'-'F' on success; 0 on failure.
+*/
 char charHex (int iDigit);
 
+/** Converts a hex digit to the corresponding integer
+    @param cDigit one of '0'-'9', 'A'-'F' or 'a'-'f'
+    @return an integer from 0 to 15 on success; -1 on failure.
+*/
+int charUnHex (char cDigit);
+
+// NIKB TODO cleanup this function and reduce the need for the many overloads
+//           it has in various places.
 template<class Iterator>
 std::string strHex (Iterator first, int iSize)
 {

src/ripple/types/impl/strHex.cpp

@@ -21,7 +21,43 @@ namespace ripple {
 
 char charHex (int iDigit)
 {
-    return iDigit < 10 ? '0' + iDigit : 'A' - 10 + iDigit;
+    if (iDigit >= 0)
+    {
+        if(iDigit < 10)
+            return '0' + iDigit;
+        if(iDigit < 16)
+            return 'A' - 10 + iDigit;
+    }
+    
+    return 0;
+}
+
+int charUnHex (char cDigit)
+{
+    struct HexTab
+    {
+        int hex[256];
+
+        HexTab ()
+        {
+            std::fill (std::begin (hex), std::end (hex), -1);
+            for (int i = 0; i < 10; ++i)
+                hex ['0'+i] = i;
+            for (int i = 0; i < 6; ++i)
+            {
+                hex ['A'+i] = 10 + i;
+                hex ['a'+i] = 10 + i;
+            }
+        }
+        int operator[] (int i) const
+        {
+           return hex[i];
+        }
+    };
+
+    static HexTab xtab;
+    
+    return xtab[cDigit];
 }
 
 }

src/ripple_app/rpc/RPCHandler.cpp

@@ -2007,15 +2007,12 @@ Json::Value RPCHandler::doSubmit (Json::Value params, Resource::Charge& loadType
 
     Json::Value                 jvResult;
 
-    Blob    vucBlob (strUnHex (params["tx_blob"].asString ()));
+    std::pair<Blob, bool> ret(strUnHex (params["tx_blob"].asString ()));
 
-    if (!vucBlob.size ())
-    {
+    if (!ret.second || !ret.first.size ())
         return rpcError (rpcINVALID_PARAMS);
-    }
 
-
-    Serializer                  sTrans (vucBlob);
+    Serializer                  sTrans (ret.first);
     SerializerIterator          sitTrans (sTrans);
 
     SerializedTransaction::pointer stpTrans;

src/ripple_basics/utility/StringUtilities.cpp

@@ -58,34 +58,6 @@ std::string strprintf (const char* format, ...)
     return str;
 }
 
-int charUnHex (char cDigit)
-{
-    struct HexTab
-    {
-        int hex[256];
-
-        HexTab ()
-        {
-            std::fill (std::begin (hex), std::end (hex), -1);
-            for (int i = 0; i < 10; ++i)
-                hex ['0'+i] = i;
-            for (int i = 0; i < 6; ++i)
-            {
-                hex ['A'+i] = 10 + i;
-                hex ['a'+i] = 10 + i;
-            }
-        }
-        int operator[] (int i) const
-        {
-           return hex[i];
-        }
-    };
-
-    static HexTab xtab;
-    
-    return xtab[cDigit];
-}
-
 // NIKB NOTE: This function is only used by strUnHex (const std::string& strSrc)
 // which results in a pointless copy from std::string into std::vector. Should
 // we just scrap this function altogether?
@@ -130,22 +102,32 @@ int strUnHex (std::string& strDst, const std::string& strSrc)
     return strDst.size ();
 }
 
-Blob strUnHex (const std::string& strSrc)
+std::pair<Blob, bool> strUnHex (const std::string& strSrc)
 {
     std::string strTmp;
 
     if (strUnHex (strTmp, strSrc) == -1)
-        return Blob ();
+        return std::make_pair (Blob (), false);
 
-    return strCopy (strTmp);
+    return std::make_pair(strCopy (strTmp), true);
 }
 
 uint64_t uintFromHex (const std::string& strSrc)
 {
-    uint64_t    uValue = 0;
+    uint64_t uValue (0);
 
-    BOOST_FOREACH (char c, strSrc)
-    uValue = (uValue << 4) | charUnHex (c);
+    if (strSrc.size () > 16)
+        throw std::invalid_argument("overlong 64-bit value");
+
+    for (auto c : strSrc)
+    {
+        int ret = charUnHex (c);
+
+        if (ret == -1)
+            throw std::invalid_argument("invalid hex digit");
+
+        uValue = (uValue << 4) | ret;
+    }
 
     return uValue;
 }

src/ripple_basics/utility/StringUtilities.h

@@ -27,60 +27,18 @@
 
 // Ripple specific constant used for parsing qualities and other things
 //
+// NIKB TODO Why is this here instead of somewhere more sensible? What 
+// "other things" is this being used for?
 #define QUALITY_ONE         1000000000  // 10e9
 
 //------------------------------------------------------------------------------
 
-// Terminal output color codes
-#define vt_f_black          "\033[30m"
-#define vt_f_red            "\033[31m"
-#define vt_f_green          "\033[32m"
-#define vt_f_yellow         "\033[33m"
-#define vt_f_blue           "\033[34m"
-#define vt_f_megenta        "\033[35m"
-#define vt_f_cyan           "\033[36m"
-#define vt_f_white          "\033[37m"
-#define vt_f_default        "\033[39m"
-
-#define vt_b_black          "\033[40m"
-#define vt_b_red            "\033[41m"
-#define vt_b_green          "\033[42m"
-#define vt_b_yellow         "\033[43m"
-#define vt_b_blue           "\033[44m"
-#define vt_b_megenta        "\033[45m"
-#define vt_b_cyan           "\033[46m"
-#define vt_b_white          "\033[47m"
-#define vt_b_default        "\033[49m"
-
-#define vt_f_bold_black    "\033[1m\033[30m"
-#define vt_f_bold_red      "\033[1m\033[31m"
-#define vt_f_bold_green    "\033[1m\033[32m"
-#define vt_f_bold_yellow   "\033[1m\033[33m"
-#define vt_f_bold_blue     "\033[1m\033[34m"
-#define vt_f_bold_megenta  "\033[1m\033[35m"
-#define vt_f_bold_cyan     "\033[1m\033[36m"
-#define vt_f_bold_white    "\033[1m\033[37m"
-#define vt_f_bold_default  "\033[1m\033[39m"
-
-#define vt_bold             "\033[1m"
-#define vt_dim              "\033[2m"     // does not work for xterm
-#define vt_normal           "\033[22m"    // intensity
-
-#define vt_n_enable         "\033[7m"     // negative
-#define vt_n_disable        "\033[27m"
-
-#define vt_u_single         "\033[4m"     // underline
-#define vt_u_double         "\033[21m"    // does not work for xterm
-#define vt_u_disable        "\033[24m"
-
-#define vt_reset    vt_f_default vt_b_default vt_normal vt_n_disable vt_u_disable
-
-//------------------------------------------------------------------------------
-
 extern std::string strprintf (const char* format, ...);
 
 extern std::string urlEncode (const std::string& strSrc);
 
+// NIKB TODO remove this function - it's only used for some logging in the UNL
+//           code which can be trivially rewritten.
 template<class Iterator>
 std::string strJoin (Iterator first, Iterator last, std::string strSeperator)
 {
@@ -94,6 +52,7 @@ std::string strJoin (Iterator first, Iterator last, std::string strSeperator)
     return ossValues.str ();
 }
 
+// NIKB TODO Remove the need for all these overloads. Move them out of here.
 inline const std::string strHex (const std::string& strSrc)
 {
     return strHex (strSrc.begin (), strSrc.size ());
@@ -143,12 +102,11 @@ inline static std::string sqlEscape (Blob const& vecSrc)
     return j;
 }
 
-int charUnHex (char cDigit);
 int strUnHex (std::string& strDst, const std::string& strSrc);
 
 uint64_t uintFromHex (const std::string& strSrc);
 
-Blob strUnHex (const std::string& strSrc);
+std::pair<Blob, bool> strUnHex (const std::string& strSrc);
 
 Blob strCopy (const std::string& strSrc);
 std::string strCopy (Blob const& vucSrc);

src/ripple_data/protocol/STParsedJSON.cpp

@@ -383,8 +383,12 @@ bool STParsedJSON::parse (std::string const& json_name,
 
             try
             {
-                data.push_back (new STVariableLength (field,
-                    strUnHex (value.asString ())));
+                std::pair<Blob, bool> ret(strUnHex (value.asString ()));
+
+                if (!ret.second)
+                    throw std::invalid_argument ("invalid data");
+
+                data.push_back (new STVariableLength (field, ret.first));
             }
             catch (...)
             {