Skip to content

Commit

Permalink
json: drop exceptions from the json sanitizer (#36000)
Browse files Browse the repository at this point in the history
<!--
!!!ATTENTION!!!

If you are fixing *any* crash or *any* potential security issue, *do
not*
open a pull request in this repo. Please report the issue via emailing
[email protected] where the issue will be triaged
appropriately.
Thank you in advance for helping to keep Envoy secure.

!!!ATTENTION!!!

For an explanation of how to fill out the fields, please see the
relevant section
in
[PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md)
-->

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional [API
Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):]

---------

Signed-off-by: Joshua Marantz <[email protected]>
  • Loading branch information
jmarantz authored Sep 8, 2024
1 parent 6618b3c commit 356499d
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 5 deletions.
1 change: 1 addition & 0 deletions source/common/json/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ envoy_cc_library(
":json_internal_lib",
"//source/common/common:assert_lib",
"//source/common/common:thread_lib",
"@utf8_range//:utf8_validity",
],
)

Expand Down
2 changes: 1 addition & 1 deletion source/common/json/json_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -787,7 +787,7 @@ ObjectSharedPtr Factory::loadFromProtobufStruct(const ProtobufWkt::Struct& proto

std::string Factory::serialize(absl::string_view str) {
nlohmann::json j(str);
return j.dump();
return j.dump(-1, ' ', false, nlohmann::detail::error_handler_t::replace);
}

std::vector<uint8_t> Factory::jsonToMsgpack(const std::string& json_string) {
Expand Down
7 changes: 3 additions & 4 deletions source/common/json/json_sanitizer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "source/common/json/json_internal.h"

#include "absl/strings/str_format.h"
#include "utf8_validity.h"

namespace Envoy {
namespace Json {
Expand Down Expand Up @@ -65,7 +66,7 @@ absl::string_view sanitize(std::string& buffer, absl::string_view str) {
if (need_slow == 0) {
return str; // Fast path, should be executed most of the time.
}
TRY_ASSERT_MAIN_THREAD {
if (utf8_range::IsStructurallyValid(str)) {
// The Nlohmann JSON library supports serialization and is not too slow. A
// hand-rolled sanitizer can be a little over 2x faster at the cost of added
// production complexity. The main drawback is that this code cannot be used
Expand All @@ -74,9 +75,7 @@ absl::string_view sanitize(std::string& buffer, absl::string_view str) {
// adds complexity to the production code base.
buffer = Nlohmann::Factory::serialize(str);
return stripDoubleQuotes(buffer);
}
END_TRY
catch (std::exception&) {
} else {
// If Nlohmann throws an error, emit a hex escape for any character
// requiring it. This can occur for invalid utf-8 sequences, and we don't
// want to crash the server if such a sequence makes its way into a string
Expand Down
5 changes: 5 additions & 0 deletions test/common/json/json_sanitizer_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ DEFINE_FUZZER(const uint8_t* buf, size_t len) {
absl::string_view proto_sanitized = Envoy::Json::stripDoubleQuotes(buffer2);
RELEASE_ASSERT(Envoy::Json::TestUtil::utf8Equivalent(sanitized, proto_sanitized, errmsg),
errmsg);
} else {
std::string decoded, errmsg;
EXPECT_TRUE(Json::TestUtil::decodeEscapedJson(sanitized, decoded, errmsg))
<< input << ": " << errmsg;
EXPECT_EQ(input, decoded);
}
}
}
Expand Down

0 comments on commit 356499d

Please sign in to comment.