Skip to content

Commit

Permalink
Fix possible C stack overflow with json.decode
Browse files Browse the repository at this point in the history
  • Loading branch information
Sainan authored and well-in-that-case committed Jan 17, 2025
1 parent 23fb606 commit ad25ead
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 39 deletions.
11 changes: 10 additions & 1 deletion src/ljson.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,16 @@ static int encode(lua_State* L) {
static int decode(lua_State* L)
{
int flags = (int)luaL_optinteger(L, 2, 0);
if (auto root = soup::json::decode(pluto_checkstring(L, 1)))
soup::UniquePtr<soup::JsonNode> root;
try
{
root = soup::json::decode(pluto_checkstring(L, 1));
}
catch (const std::exception& e)
{
luaL_error(L, e.what());
}
if (root)
{
pushFromJson(L, *root, flags);
return 1;
Expand Down
4 changes: 2 additions & 2 deletions src/vendor/Soup/soup/JsonArray.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ NAMESPACE_SOUP
{
}

JsonArray::JsonArray(const char*& c)
JsonArray::JsonArray(const char*& c, int max_depth)
: JsonArray()
{
while (true)
{
json::handleLeadingSpace(c);
auto val = json::decode(c);
auto val = json::decode(c, max_depth);
if (!val)
{
break;
Expand Down
2 changes: 1 addition & 1 deletion src/vendor/Soup/soup/JsonArray.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ NAMESPACE_SOUP
std::vector<UniquePtr<JsonNode>> children{};

explicit JsonArray() noexcept;
explicit JsonArray(const char*& c);
explicit JsonArray(const char*& c, int max_depth);

void encodeAndAppendTo(std::string& str) const SOUP_EXCAL final;
void encodePrettyAndAppendTo(std::string& str, unsigned depth = 0) const SOUP_EXCAL;
Expand Down
6 changes: 3 additions & 3 deletions src/vendor/Soup/soup/JsonObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ NAMESPACE_SOUP
{
}

JsonObject::JsonObject(const char*& c) noexcept
JsonObject::JsonObject(const char*& c, int max_depth) noexcept
: JsonObject()
{
while (true)
Expand All @@ -28,12 +28,12 @@ NAMESPACE_SOUP
{
break;
}
auto key = json::decode(c);
auto key = json::decode(c, max_depth);
while (string::isSpace(*c) || *c == ':')
{
++c;
}
auto val = json::decode(c);
auto val = json::decode(c, max_depth);
if (!key || !val)
{
break;
Expand Down
2 changes: 1 addition & 1 deletion src/vendor/Soup/soup/JsonObject.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ NAMESPACE_SOUP
Container children{};

explicit JsonObject() noexcept;
explicit JsonObject(const char*& c) noexcept;
explicit JsonObject(const char*& c, int max_depth) noexcept;

void encodeAndAppendTo(std::string& str) const SOUP_EXCAL final;
void encodePrettyAndAppendTo(std::string& str, unsigned depth = 0) const SOUP_EXCAL;
Expand Down
35 changes: 11 additions & 24 deletions src/vendor/Soup/soup/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@

NAMESPACE_SOUP
{
UniquePtr<JsonNode> json::decode(const std::string& data)
UniquePtr<JsonNode> json::decode(const std::string& data, int max_depth)
{
if (data.empty())
{
return {};
}
const char* c = data.c_str();
return decode(c);
return decode(c, max_depth);
}

UniquePtr<JsonNode> json::decode(const char*& c)
UniquePtr<JsonNode> json::decode(const char*& c, int max_depth)
{
SOUP_ASSERT(max_depth != 0, "Depth limit exceeded");

handleLeadingSpace(c);

switch (*c)
Expand All @@ -34,11 +36,11 @@ NAMESPACE_SOUP

case '[':
++c;
return soup::make_unique<JsonArray>(c);
return soup::make_unique<JsonArray>(c, max_depth - 1);

case '{':
++c;
return soup::make_unique<JsonObject>(c);
return soup::make_unique<JsonObject>(c, max_depth - 1);
}

std::string buf{};
Expand Down Expand Up @@ -122,22 +124,7 @@ NAMESPACE_SOUP
return {};
}

void json::decode(UniquePtr<JsonNode>& out, const std::string& data)
{
out = decode(data);
}

UniquePtr<JsonNode> json::decodeForDedicatedVariable(const std::string& data)
{
return decode(data);
}

void json::binaryDecode(UniquePtr<JsonNode>& out, Reader& r)
{
out = binaryDecodeForDedicatedVariable(r);
}

UniquePtr<JsonNode> json::binaryDecodeForDedicatedVariable(Reader& r)
UniquePtr<JsonNode> json::binaryDecode(Reader& r)
{
uint8_t b;
if (r.u8(b))
Expand Down Expand Up @@ -190,7 +177,7 @@ NAMESPACE_SOUP
{
UniquePtr<JsonNode> node;

if (binaryDecode(node, r), !node)
if (node = binaryDecode(r), !node)
{
break;
}
Expand All @@ -207,11 +194,11 @@ NAMESPACE_SOUP
UniquePtr<JsonNode> key;
UniquePtr<JsonNode> val;

if (binaryDecode(key, r), !key)
if (key = binaryDecode(r), !key)
{
break;
}
if (binaryDecode(val, r), !val)
if (val = binaryDecode(r), !val)
{
break;
}
Expand Down
10 changes: 3 additions & 7 deletions src/vendor/Soup/soup/json.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,11 @@ NAMESPACE_SOUP
// Don't do json::decode(...)->... because:
// 1. UniquePtr manages the lifetime, by using the return value as a temporary like that, you will use-after-free.
// 2. The returned UniquePtr might be default-constructed in case of a parse error, so you'd be dereferencing a nullptr.
[[nodiscard]] static UniquePtr<JsonNode> decode(const std::string& data);
[[nodiscard]] static UniquePtr<JsonNode> decode(const char*& c);

[[deprecated]] static void decode(UniquePtr<JsonNode>& out, const std::string& data);
[[deprecated]] static UniquePtr<JsonNode> decodeForDedicatedVariable(const std::string& data);
[[nodiscard]] static UniquePtr<JsonNode> decode(const std::string& data, int max_depth = 100);
[[nodiscard]] static UniquePtr<JsonNode> decode(const char*& c, int max_depth);

// specific to soup
static void binaryDecode(UniquePtr<JsonNode>& out, Reader& r);
[[nodiscard]] static UniquePtr<JsonNode> binaryDecodeForDedicatedVariable(Reader& r);
[[nodiscard]] static UniquePtr<JsonNode> binaryDecode(Reader& r);

// internal
static void handleLeadingSpace(const char*& c);
Expand Down
3 changes: 3 additions & 0 deletions testes/pluto/basic.pluto
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,9 @@ do
assert(json.decode([[{"a":null]]).a == nil)
assert(json.decode([[{"a":null}]], json.withnull).a == json.null)

-- Don't want a C stack overflow here
assert(not pcall(|| -> require"json".decode("[":rep(10000).."]":rep(10000))))

do
local t = {
__order = { "a", "b", "c" },
Expand Down

0 comments on commit ad25ead

Please sign in to comment.