Perform SqlHexDecode transformation in-place

- Validate buffer size before accessing data. The previous
  implementation would only check that there was a character available
  in the buffer but could continue processing/reading characters from
  an hex representation without checking bounds.
- Removed inplace & mytolower helper functions from the class, as
  they're only referenced by the implementation.
- Removed duplicate VALID_HEX & ISODIGIT macros, already in
  src/utils/string.h.

src/actions/transformations/sql_hex_decode.cc

@@ -23,79 +23,50 @@
 namespace modsecurity::actions::transformations {
 
 
-#ifndef VALID_HEX
+static inline int mytolower(int ch) {
-#define VALID_HEX(X) (((X >= '0') && (X <= '9')) \
+    if (ch >= 'A' && ch <= 'Z')
-    || ((X >= 'a') && (X <= 'f')) \
+        return ('a' + ch - 'A');
-    || ((X >= 'A') && (X <= 'F')))
+    else
-#endif
+        return ch;
-#ifndef ISODIGIT
-#define ISODIGIT(X) ((X >= '0') && (X <= '7'))
-#endif
-
-bool SqlHexDecode::transform(std::string &value, const Transaction *trans) const {
-    std::string ret;
-    unsigned char *input;
-    int size = 0;
-
-    input = reinterpret_cast<unsigned char *>
-        (malloc(sizeof(char) * value.length()+1));
-
-    if (input == NULL) {
-        return "";
-    }
-
-    memcpy(input, value.c_str(), value.length()+1);
-
-    size = inplace(input, value.length());
-
-    ret.assign(reinterpret_cast<char *>(input), size);
-    free(input);
-
-    const auto changed = ret != value;
-    value = ret;
-    return changed;
 }
 
-
+static inline bool inplace(std::string &value) {
-int SqlHexDecode::inplace(unsigned char *data, int len) {
+    if (value.empty()) {
-    unsigned char *d, *begin = data;
+        return false;
-    int count = 0;
-
-    if ((data == NULL) || (len == 0)) {
-        return 0;
     }
 
-    for (d = data; (++count < len) && *data; *d++ = *data++) {
+    auto d = reinterpret_cast<unsigned char*>(value.data());
-        if (*data != '0') {
+    const unsigned char *data = d;
-            continue;
+    const auto end = data + value.size();
-        }
-        ++data;
-        ++count;
-        if (mytolower(*data) != 'x') {
-            data--;
-            count--;
-            continue;
-        }
 
-        data++;
+    bool changed = false;
-        ++count;
 
-        // Do we need to keep "0x" if no hexa after?
+    while (data < end) {
-        if (!VALID_HEX(data[0]) || !VALID_HEX(data[1])) {
+        if (data + 3 < end
-            data -= 2;
+            && *data == '0'
-            count -= 2;
+            && mytolower(*(data + 1)) == 'x'
-            continue;
+            && VALID_HEX(*(data + 2)) && VALID_HEX(*(data + 3))) {
+            data += 2;  // skip '0x'
+            do {
+                *d++ = utils::string::x2c(data);
+                data += 2;
+            } while ((data + 1 < end) && VALID_HEX(*data) && VALID_HEX(*(data + 1)));
+            changed = true;
         }
-
+        else {
-        while (VALID_HEX(data[0]) && VALID_HEX(data[1])) {
+            *d++ = *data++;
-            *d++ = utils::string::x2c(data);
-            data += 2;
-            count += 2;
         }
     }
 
     *d = '\0';
-    return strlen(reinterpret_cast<char *>(begin));
+
+    value.resize(d - reinterpret_cast<const unsigned char*>(value.c_str()));
+    return changed;
+}
+
+
+bool SqlHexDecode::transform(std::string &value, const Transaction *trans) const {
+    return inplace(value);
 }
 
 

src/actions/transformations/sql_hex_decode.h

@@ -26,15 +26,6 @@ class SqlHexDecode : public Transformation {
         : Transformation(action) { }
 
     bool transform(std::string &value, const Transaction *trans) const override;
-
-    static int inplace(unsigned char *data, int len);
-
-    static int mytolower(int ch) {
-        if (ch >= 'A' && ch <= 'Z')
-            return ('a' + ch - 'A');
-        else
-            return ch;
-    }
 };
 
 }  // namespace modsecurity::actions::transformations