Skip to content

Commit 5ebddcb

Browse files
Fix Any recursion depth bypass in Python json_format.ParseDict (#25239) (#25587)
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 Closes #25239 COPYBARA_INTEGRATE_REVIEW=#25239 from aviralgarg05:fix-any-recursion-depth-bypass 3cbbcbe PiperOrigin-RevId: 862740421 Co-authored-by: Aviral Garg <gargaviral99@gmail.com>
1 parent f340323 commit 5ebddcb

File tree

2 files changed

+110
-6
lines changed

2 files changed

+110
-6
lines changed

python/google/protobuf/internal/json_format_test.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1630,6 +1630,107 @@ def testNestedRecursiveLimit(self):
16301630
'{"payload": {}, "child": {"child":{}}}', message, max_recursion_depth=3
16311631
)
16321632

1633+
def testAnyRecursionDepthEnforcement(self):
1634+
"""Test that nested Any messages respect max_recursion_depth limit."""
1635+
# Test that deeply nested Any messages raise ParseError instead of
1636+
# bypassing the recursion limit. This prevents DoS via nested Any.
1637+
message = any_pb2.Any()
1638+
1639+
# Create nested Any structure that should exceed depth limit
1640+
# With max_recursion_depth=5, we can nest 4 Any messages
1641+
# (depth 1 = outer Any, depth 2-4 = nested Anys, depth 5 = final value)
1642+
nested_any = {
1643+
'@type': 'type.googleapis.com/google.protobuf.Any',
1644+
'value': {
1645+
'@type': 'type.googleapis.com/google.protobuf.Any',
1646+
'value': {
1647+
'@type': 'type.googleapis.com/google.protobuf.Any',
1648+
'value': {
1649+
'@type': 'type.googleapis.com/google.protobuf.Any',
1650+
'value': {
1651+
'@type': 'type.googleapis.com/google.protobuf.Any',
1652+
'value': {},
1653+
},
1654+
},
1655+
},
1656+
},
1657+
}
1658+
1659+
# Should raise ParseError due to exceeding max depth, not RecursionError
1660+
self.assertRaisesRegex(
1661+
json_format.ParseError,
1662+
'Message too deep. Max recursion depth is 5',
1663+
json_format.ParseDict,
1664+
nested_any,
1665+
message,
1666+
max_recursion_depth=5,
1667+
)
1668+
1669+
# Verify that Any messages within the limit can be parsed successfully
1670+
# With max_recursion_depth=5, we can nest up to 4 Any messages
1671+
shallow_any = {
1672+
'@type': 'type.googleapis.com/google.protobuf.Any',
1673+
'value': {
1674+
'@type': 'type.googleapis.com/google.protobuf.Any',
1675+
'value': {
1676+
'@type': 'type.googleapis.com/google.protobuf.Any',
1677+
'value': {
1678+
'@type': 'type.googleapis.com/google.protobuf.Any',
1679+
'value': {},
1680+
},
1681+
},
1682+
},
1683+
}
1684+
json_format.ParseDict(shallow_any, message, max_recursion_depth=5)
1685+
1686+
def testAnyRecursionDepthBoundary(self):
1687+
"""Test recursion depth boundary behavior (exclusive upper limit)."""
1688+
message = any_pb2.Any()
1689+
1690+
# Create nested Any at depth exactly 4 (should succeed with max_recursion_depth=5)
1691+
depth_4_any = {
1692+
'@type': 'type.googleapis.com/google.protobuf.Any',
1693+
'value': {
1694+
'@type': 'type.googleapis.com/google.protobuf.Any',
1695+
'value': {
1696+
'@type': 'type.googleapis.com/google.protobuf.Any',
1697+
'value': {
1698+
'@type': 'type.googleapis.com/google.protobuf.Any',
1699+
'value': {},
1700+
},
1701+
},
1702+
},
1703+
}
1704+
# This should succeed: depth 4 < max_recursion_depth 5
1705+
json_format.ParseDict(depth_4_any, message, max_recursion_depth=5)
1706+
1707+
# Create nested Any at depth exactly 5 (should fail with max_recursion_depth=5)
1708+
depth_5_any = {
1709+
'@type': 'type.googleapis.com/google.protobuf.Any',
1710+
'value': {
1711+
'@type': 'type.googleapis.com/google.protobuf.Any',
1712+
'value': {
1713+
'@type': 'type.googleapis.com/google.protobuf.Any',
1714+
'value': {
1715+
'@type': 'type.googleapis.com/google.protobuf.Any',
1716+
'value': {
1717+
'@type': 'type.googleapis.com/google.protobuf.Any',
1718+
'value': {},
1719+
},
1720+
},
1721+
},
1722+
},
1723+
}
1724+
# This should fail: depth 5 == max_recursion_depth 5 (exclusive limit)
1725+
self.assertRaisesRegex(
1726+
json_format.ParseError,
1727+
'Message too deep. Max recursion depth is 5',
1728+
json_format.ParseDict,
1729+
depth_5_any,
1730+
message,
1731+
max_recursion_depth=5,
1732+
)
1733+
16331734
def testJsonNameConflictSerilize(self):
16341735
message = more_messages_pb2.ConflictJsonName(value=2)
16351736
self.assertEqual(

python/google/protobuf/json_format.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,10 @@ def ConvertMessage(self, value, message, path):
521521
Raises:
522522
ParseError: In case of convert problems.
523523
"""
524+
# Increment recursion depth at message entry. The max_recursion_depth limit
525+
# is exclusive: a depth value equal to max_recursion_depth will trigger an
526+
# error. For example, with max_recursion_depth=5, nesting up to depth 4 is
527+
# allowed, but attempting depth 5 raises ParseError.
524528
self.recursion_depth += 1
525529
if self.recursion_depth > self.max_recursion_depth:
526530
raise ParseError(
@@ -725,12 +729,11 @@ def _ConvertAnyMessage(self, value, message, path):
725729
value['value'], sub_message, '{0}.value'.format(path)
726730
)
727731
elif full_name in _WKTJSONMETHODS:
728-
methodcaller(
729-
_WKTJSONMETHODS[full_name][1],
730-
value['value'],
731-
sub_message,
732-
'{0}.value'.format(path),
733-
)(self)
732+
# For well-known types (including nested Any), use ConvertMessage
733+
# to ensure recursion depth is properly tracked
734+
self.ConvertMessage(
735+
value['value'], sub_message, '{0}.value'.format(path)
736+
)
734737
else:
735738
del value['@type']
736739
self._ConvertFieldValuePair(value, sub_message, path)

0 commit comments

Comments
 (0)