218 lines
8.5 KiB
Diff
218 lines
8.5 KiB
Diff
From 38c2e552baa3eed0c50d8a1d878ffc1634b0768f Mon Sep 17 00:00:00 2001
|
|
From: aviralgarg05 <gargaviral99@gmail.com>
|
|
Date: Fri, 9 Jan 2026 20:59:10 +0530
|
|
Subject: [PATCH] Fix Any recursion depth bypass in Python
|
|
json_format.ParseDict
|
|
|
|
This fixes a security vulnerability where nested google.protobuf.Any messages
|
|
could bypass the max_recursion_depth limit, potentially leading to denial of
|
|
service via stack overflow.
|
|
|
|
The root cause was that _ConvertAnyMessage() was calling itself recursively
|
|
via methodcaller() for nested well-known types, bypassing the recursion depth
|
|
tracking in ConvertMessage().
|
|
|
|
The fix routes well-known type parsing through ConvertMessage() to ensure
|
|
proper recursion depth accounting for all message types including nested Any.
|
|
|
|
Fixes #25070
|
|
---
|
|
.../protobuf/internal/json_format_test.py | 150 +++++++++++++++++-
|
|
python/google/protobuf/json_format.py | 12 +-
|
|
2 files changed, 157 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/python/google/protobuf/internal/json_format_test.py b/python/google/protobuf/internal/json_format_test.py
|
|
index b4f516d83..74c45f217 100644
|
|
--- a/python/google/protobuf/internal/json_format_test.py
|
|
+++ b/python/google/protobuf/internal/json_format_test.py
|
|
@@ -1295,8 +1295,154 @@ class JsonFormatTest(JsonFormatBase):
|
|
message,
|
|
max_recursion_depth=3)
|
|
# The following one can pass
|
|
- json_format.Parse('{"payload": {}, "child": {"child":{}}}',
|
|
- message, max_recursion_depth=3)
|
|
+ json_format.Parse(
|
|
+ '{"payload": {}, "child": {"child":{}}}', message, max_recursion_depth=3
|
|
+ )
|
|
+
|
|
+ def testAnyRecursionDepthEnforcement(self):
|
|
+ """Test that nested Any messages respect max_recursion_depth limit."""
|
|
+ # Test that deeply nested Any messages raise ParseError instead of
|
|
+ # bypassing the recursion limit. This prevents DoS via nested Any.
|
|
+ message = any_pb2.Any()
|
|
+
|
|
+ # Create nested Any structure that should exceed depth limit
|
|
+ # With max_recursion_depth=5, we can nest 4 Any messages
|
|
+ # (depth 1 = outer Any, depth 2-4 = nested Anys, depth 5 = final value)
|
|
+ nested_any = {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {},
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ }
|
|
+
|
|
+ # Should raise ParseError due to exceeding max depth, not RecursionError
|
|
+ self.assertRaisesRegex(
|
|
+ json_format.ParseError,
|
|
+ 'Message too deep. Max recursion depth is 5',
|
|
+ json_format.ParseDict,
|
|
+ nested_any,
|
|
+ message,
|
|
+ max_recursion_depth=5,
|
|
+ )
|
|
+
|
|
+ # Verify that Any messages within the limit can be parsed successfully
|
|
+ # With max_recursion_depth=5, we can nest up to 4 Any messages
|
|
+ shallow_any = {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {},
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ }
|
|
+ json_format.ParseDict(shallow_any, message, max_recursion_depth=5)
|
|
+
|
|
+ def testAnyRecursionDepthBoundary(self):
|
|
+ """Test recursion depth boundary behavior (exclusive upper limit)."""
|
|
+ message = any_pb2.Any()
|
|
+
|
|
+ # Create nested Any at depth exactly 4 (should succeed with max_recursion_depth=5)
|
|
+ depth_4_any = {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {},
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ }
|
|
+ # This should succeed: depth 4 < max_recursion_depth 5
|
|
+ json_format.ParseDict(depth_4_any, message, max_recursion_depth=5)
|
|
+
|
|
+ # Create nested Any at depth exactly 5 (should fail with max_recursion_depth=5)
|
|
+ depth_5_any = {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {
|
|
+ '@type': 'type.googleapis.com/google.protobuf.Any',
|
|
+ 'value': {},
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ },
|
|
+ }
|
|
+ # This should fail: depth 5 == max_recursion_depth 5 (exclusive limit)
|
|
+ self.assertRaisesRegex(
|
|
+ json_format.ParseError,
|
|
+ 'Message too deep. Max recursion depth is 5',
|
|
+ json_format.ParseDict,
|
|
+ depth_5_any,
|
|
+ message,
|
|
+ max_recursion_depth=5,
|
|
+ )
|
|
+
|
|
+ def testJsonNameConflictSerilize(self):
|
|
+ message = more_messages_pb2.ConflictJsonName(value=2)
|
|
+ self.assertEqual(
|
|
+ json.loads('{"old_value": 2}'),
|
|
+ json.loads(json_format.MessageToJson(message)),
|
|
+ )
|
|
+
|
|
+ new_message = more_messages_pb2.ConflictJsonName(new_value=2)
|
|
+ self.assertEqual(
|
|
+ json.loads('{"value": 2}'),
|
|
+ json.loads(json_format.MessageToJson(new_message)),
|
|
+ )
|
|
+
|
|
+ def testJsonNameConflictParse(self):
|
|
+ message = more_messages_pb2.ConflictJsonName()
|
|
+ json_format.Parse('{"value": 2}', message)
|
|
+ self.assertEqual(2, message.new_value)
|
|
+ self.assertEqual(0, message.value)
|
|
+
|
|
+ def testJsonNameConflictRoundTrip(self):
|
|
+ message = more_messages_pb2.ConflictJsonName(value=2)
|
|
+ parsed_message = more_messages_pb2.ConflictJsonName()
|
|
+ json_string = json_format.MessageToJson(message)
|
|
+ json_format.Parse(json_string, parsed_message)
|
|
+ self.assertEqual(message, parsed_message)
|
|
+
|
|
+ new_message = more_messages_pb2.ConflictJsonName(new_value=2)
|
|
+ new_parsed_message = more_messages_pb2.ConflictJsonName()
|
|
+ json_string = json_format.MessageToJson(new_message)
|
|
+ json_format.Parse(json_string, new_parsed_message)
|
|
+ self.assertEqual(new_message, new_parsed_message)
|
|
+
|
|
+ def testOtherParseErrors(self):
|
|
+ self.CheckError(
|
|
+ '9',
|
|
+ "Failed to parse JSON: TypeError: 'int' object is not iterable.",
|
|
+ )
|
|
+
|
|
+ def testManyRecursionsRaisesParseError(self):
|
|
+ num_recursions = 1050
|
|
+ text = ('{"a":' * num_recursions) + '""' + ('}' * num_recursions)
|
|
+ with self.assertRaises(json_format.ParseError):
|
|
+ json_format.Parse(text, json_format_proto3_pb2.TestMessage())
|
|
|
|
if __name__ == '__main__':
|
|
unittest.main()
|
|
diff --git a/python/google/protobuf/json_format.py b/python/google/protobuf/json_format.py
|
|
index a04e8aef1..2fe8da64c 100644
|
|
--- a/python/google/protobuf/json_format.py
|
|
+++ b/python/google/protobuf/json_format.py
|
|
@@ -496,6 +496,10 @@ class _Parser(object):
|
|
Raises:
|
|
ParseError: In case of convert problems.
|
|
"""
|
|
+ # Increment recursion depth at message entry. The max_recursion_depth limit
|
|
+ # is exclusive: a depth value equal to max_recursion_depth will trigger an
|
|
+ # error. For example, with max_recursion_depth=5, nesting up to depth 4 is
|
|
+ # allowed, but attempting depth 5 raises ParseError.
|
|
self.recursion_depth += 1
|
|
if self.recursion_depth > self.max_recursion_depth:
|
|
raise ParseError('Message too deep. Max recursion depth is {0}'.format(
|
|
@@ -669,9 +673,11 @@ class _Parser(object):
|
|
self._ConvertWrapperMessage(value['value'], sub_message,
|
|
'{0}.value'.format(path))
|
|
elif full_name in _WKTJSONMETHODS:
|
|
- methodcaller(_WKTJSONMETHODS[full_name][1], value['value'], sub_message,
|
|
- '{0}.value'.format(path))(
|
|
- self)
|
|
+ # For well-known types (including nested Any), use ConvertMessage
|
|
+ # to ensure recursion depth is properly tracked
|
|
+ self.ConvertMessage(
|
|
+ value['value'], sub_message, '{0}.value'.format(path)
|
|
+ )
|
|
else:
|
|
del value['@type']
|
|
self._ConvertFieldValuePair(value, sub_message, path)
|
|
--
|
|
2.52.0
|
|
|