redis: fix uncaught exception in inline command hex escape parser causing full-process DoS [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT]

changelogs/current/bug_fixes/redis__inline-hex-escape-crash.rst

@@ -0,0 +1,9 @@
+Fixed a remote, unauthenticated denial-of-service in the Redis proxy filter: a single inline
+command containing a literal ``x`` before a ``\xHH`` escape (e.g. ``SET key "x\x41"``) caused the
+quoted-string decoder to mis-detect the escape marker and call ``std::stoul`` on a non-hex
+string, throwing ``std::invalid_argument`` that escaped the filter and terminated the whole
+Envoy process. The decoder no longer scans backwards for the escape marker; it now accumulates
+the two hex digits of a ``\xHH`` escape in a dedicated buffer and converts them only once both
+are present, matching Redis semantics. A truncated escape (one hex digit before the closing
+quote) is emitted literally instead of crashing, and the out-of-bounds read when ``\x`` was the
+first content of a quoted argument is eliminated.

source/extensions/filters/network/common/redis/client_impl.cc

@@ -311,6 +311,13 @@ void ClientImpl::onData(Buffer::Instance& data) {
     host_->cluster().trafficStats()->upstream_cx_protocol_error_.inc();
     host_->stats().rq_error_.inc();
     connection_->close(Network::ConnectionCloseType::NoFlush);
+  } catch (std::exception& e) {
+    ENVOY_LOG(warn, "redis client: unexpected exception while decoding upstream data: {}",
+              e.what());
+    putOutlierEvent(Upstream::Outlier::Result::ExtOriginRequestFailed);
+    host_->cluster().trafficStats()->upstream_cx_protocol_error_.inc();
+    host_->stats().rq_error_.inc();
+    connection_->close(Network::ConnectionCloseType::NoFlush);
   }
 }
 

source/extensions/filters/network/common/redis/codec_impl.cc

@@ -704,7 +704,7 @@ void DecoderImpl::parseSlice(const Buffer::RawSlice& slice) {
         throw ProtocolError("unbalanced quotes in request");
       } else if (buffer[0] == 'x') {
         state_ = State::InlineStringQuotedEscapeHex;
-        pending_value_stack_.front().value_->asString().push_back(buffer[0]);
+        inline_hex_digits_.clear();
       } else {
         char c;
         switch (buffer[0]) {
@@ -742,18 +742,34 @@ void DecoderImpl::parseSlice(const Buffer::RawSlice& slice) {
     case State::InlineStringQuotedEscapeHex: {
       ENVOY_LOG(trace, "parse slice: InlineStringQuotedEscapeHex: {}", buffer[0]);
 
+      auto& s = pending_value_stack_.front().value_->asString();
       if (!std::isxdigit(buffer[0])) {
+        // A non-hex digit terminates a (possibly incomplete) \xHH escape. Redis
+        // treats the escape marker literally: emit ``x`` followed by any hex digits
+        // already seen, then reprocess the current byte in the quoted-string state.
+        s.push_back('x');
+        s += inline_hex_digits_;
+        inline_hex_digits_.clear();
         state_ = State::InlineStringQuoted;
-        break;
+        break; // do not consume buffer[0]; reprocess in InlineStringQuoted
       }
 
-      auto& s = pending_value_stack_.front().value_->asString();
-      ASSERT((!s.empty() && s.back() == 'x') || (s.size() > 1 && s[s.size() - 2] == 'x'));
-      s.push_back(buffer[0]);
-      if (s[s.size() - 3] == 'x') {
-        char c = static_cast<char>(std::stoul(&s[s.size() - 2], nullptr, 16));
-        s.resize(s.size() - 3);
-        s.push_back(c);
+      inline_hex_digits_.push_back(buffer[0]);
+      if (inline_hex_digits_.size() == 2) {
+        // Convert the two hex digits to a single byte.
+        auto hex_val = [](char c) -> int {
+          if (c >= '0' && c <= '9') {
+            return c - '0';
+          }
+          if (c >= 'a' && c <= 'f') {
+            return c - 'a' + 10;
+          }
+          return c - 'A' + 10;
+        };
+        const char byte = static_cast<char>((hex_val(inline_hex_digits_[0]) << 4) |
+                                             hex_val(inline_hex_digits_[1]));
+        s.push_back(byte);
+        inline_hex_digits_.clear();
         state_ = State::InlineStringQuoted;
       }
 

source/extensions/filters/network/common/redis/codec_impl.h

@@ -122,6 +122,13 @@ class DecoderImpl : public Decoder, Logger::Loggable<Logger::Id::redis> {
   // cap enforced as each byte is accumulated; the terminating CR validates numeric syntax and
   // moves the buffer into ``RespValue::asString()``.
   std::string pending_double_buf_;
+  // Scratch buffer for the one-or-two hex digits of a ``\xHH`` escape currently being
+  // decoded in a double-quoted inline string. The escape marker ``x`` is NOT appended
+  // to the value; digits accumulate here and are converted to a single byte only once
+  // the second hex digit arrives (matching Redis ``\xHH`` semantics). A non-hex digit
+  // or end-of-quote before the second digit flushes the literal ``x`` followed by the
+  // digits already seen.
+  std::string inline_hex_digits_;
   uint32_t consecutive_attributes_{0}; // counts toward kMaxConsecutiveAttributes
   // Number of attribute frames currently open on pending_value_stack_. Values completing while
   // this is non-zero belong to a frame that will itself be discarded, so they must not reset

source/extensions/filters/network/redis_proxy/proxy_filter.cc

@@ -477,6 +477,21 @@ Network::FilterStatus ProxyFilter::onData(Buffer::Instance& data, bool) {
     callbacks_->connection().write(encoder_buffer_, false);
     callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);
     return Network::FilterStatus::StopIteration;
+  } catch (std::exception& e) {
+    // Defense in depth: the decoder must never let an unexpected exception escape
+    // (previously a std::invalid_argument from the inline-command hex escape parser
+    // could terminate the whole process). Treat any other exception as a protocol
+    // error and close the connection.
+    ENVOY_LOG(warn, "redis proxy: unexpected exception while decoding downstream data: {}",
+              e.what());
+    config_->stats_.downstream_cx_protocol_error_.inc();
+    Common::Redis::RespValue error;
+    error.type(Common::Redis::RespType::Error);
+    error.asString() = "downstream protocol error";
+    encoder_->encode(error, encoder_buffer_);
+    callbacks_->connection().write(encoder_buffer_, false);
+    callbacks_->connection().close(Network::ConnectionCloseType::NoFlush);
+    return Network::FilterStatus::StopIteration;
   }
 }
 

test/extensions/filters/network/common/redis/codec_impl_test.cc

@@ -836,6 +836,70 @@ TEST_F(RedisEncoderDecoderImplTest, InlineCommandQuotedInvalidEscapes) {
   EXPECT_EQ(unescaped, decoded_values_[0]->asArray()[0]);
 }
 
+// Verify that a literal 'x' before an \xHH escape does not cause a false-positive
+// hex-digit completion check (issue #46642: the old code scanned 3 characters back
+// for the escape marker, colliding with a user-supplied literal 'x').
+TEST_F(RedisEncoderDecoderImplTest, InlineCommandQuotedHexEscapeLiteralXBefore) {
+  // Input: "x\x41" → literal 'x', then \x41 = 'A' → "xA"
+  RespValue unescaped;
+  unescaped.type(RespType::BulkString);
+  unescaped.asString() = "x\x41"; // "x" + byte 0x41 = 'A'
+
+  buffer_.add("\"x\\x41\"\r\n");
+  decoder_.decode(buffer_);
+  EXPECT_EQ(1UL, decoded_values_.size());
+  EXPECT_EQ(RespType::Array, decoded_values_[0]->type());
+  EXPECT_EQ(1UL, decoded_values_[0]->asArray().size());
+  EXPECT_EQ(unescaped, decoded_values_[0]->asArray()[0]);
+}
+
+// Verify that a truncated \x escape (one hex digit, then end of quoted string)
+// does not crash. Regression test for issue #46642.
+TEST_F(RedisEncoderDecoderImplTest, InlineCommandQuotedHexEscapeTruncated) {
+  // Input: "x\x4" → three chars: 'x', 'x' (incomplete escape marker), '4' (only hex digit)
+  RespValue unescaped;
+  unescaped.type(RespType::BulkString);
+  unescaped.asString() = "xx4";
+
+  buffer_.add("\"x\\x4\"\r\n");
+  decoder_.decode(buffer_);
+  EXPECT_EQ(1UL, decoded_values_.size());
+  EXPECT_EQ(RespType::Array, decoded_values_[0]->type());
+  EXPECT_EQ(1UL, decoded_values_[0]->asArray().size());
+  EXPECT_EQ(unescaped, decoded_values_[0]->asArray()[0]);
+}
+
+// Verify that an \x escape at the start of a quoted string (no preceding chars)
+// does not cause an out-of-bounds read. Regression test for issue #46642.
+TEST_F(RedisEncoderDecoderImplTest, InlineCommandQuotedHexEscapeAtStart) {
+  // Input: "\x4" → two chars: 'x' (incomplete escape marker), '4' (only hex digit)
+  RespValue unescaped;
+  unescaped.type(RespType::BulkString);
+  unescaped.asString() = "x4";
+
+  buffer_.add("\"\\x4\"\r\n");
+  decoder_.decode(buffer_);
+  EXPECT_EQ(1UL, decoded_values_.size());
+  EXPECT_EQ(RespType::Array, decoded_values_[0]->type());
+  EXPECT_EQ(1UL, decoded_values_[0]->asArray().size());
+  EXPECT_EQ(unescaped, decoded_values_[0]->asArray()[0]);
+}
+
+// Verify that multiple consecutive \xHH escapes decode correctly.
+TEST_F(RedisEncoderDecoderImplTest, InlineCommandQuotedHexEscapeMultiple) {
+  // Input: "\x41\x42\x43" → "ABC"
+  RespValue unescaped;
+  unescaped.type(RespType::BulkString);
+  unescaped.asString() = "ABC";
+
+  buffer_.add("\"\\x41\\x42\\x43\"\r\n");
+  decoder_.decode(buffer_);
+  EXPECT_EQ(1UL, decoded_values_.size());
+  EXPECT_EQ(RespType::Array, decoded_values_[0]->type());
+  EXPECT_EQ(1UL, decoded_values_[0]->asArray().size());
+  EXPECT_EQ(unescaped, decoded_values_[0]->asArray()[0]);
+}
+
 TEST_F(RedisEncoderDecoderImplTest, InlineCommandSingleQuotedCommand) {
   RespValue echo;
   echo.type(RespType::BulkString);