From f096ee6a3091abaa2ee8d3113c7c7ad936353173 Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Mon, 26 Jan 2026 10:12:24 -0800 Subject: [PATCH] Internal changes COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/25239 from aviralgarg05:fix-any-recursion-depth-bypass 3cbbcbea142593d3afd2ceba2db14b05660f62f4 FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/25239 from aviralgarg05:fix-any-recursion-depth-bypass 3cbbcbea142593d3afd2ceba2db14b05660f62f4 PiperOrigin-RevId: 861246215 --- .../protobuf/internal/json_format_test.py | 101 ++++++++++++++++++ python/google/protobuf/json_format.py | 15 +-- 2 files changed, 110 insertions(+), 6 deletions(-) diff --git a/python/google/protobuf/internal/json_format_test.py b/python/google/protobuf/internal/json_format_test.py index f7c74739d3db9..4dc8359fb5b12 100644 --- a/python/google/protobuf/internal/json_format_test.py +++ b/python/google/protobuf/internal/json_format_test.py @@ -1760,6 +1760,107 @@ def testNestedRecursiveLimit(self): '{"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( diff --git a/python/google/protobuf/json_format.py b/python/google/protobuf/json_format.py index 56c29cccc95cb..9262abafa7cbc 100644 --- a/python/google/protobuf/json_format.py +++ b/python/google/protobuf/json_format.py @@ -524,6 +524,10 @@ def ConvertMessage(self, value, message, path): 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( @@ -744,12 +748,11 @@ def _ConvertAnyMessage(self, value, message, path): 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'] try: