Skip to content

Commit 3cbbcbe

Browse files
committed
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
1 parent eba53e8 commit 3cbbcbe

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
@@ -1760,6 +1760,107 @@ def testNestedRecursiveLimit(self):
17601760
'{"payload": {}, "child": {"child":{}}}', message, max_recursion_depth=3
17611761
)
17621762

1763+
def testAnyRecursionDepthEnforcement(self):
1764+
"""Test that nested Any messages respect max_recursion_depth limit."""
1765+
# Test that deeply nested Any messages raise ParseError instead of
1766+
# bypassing the recursion limit. This prevents DoS via nested Any.
1767+
message = any_pb2.Any()
1768+
1769+
# Create nested Any structure that should exceed depth limit
1770+
# With max_recursion_depth=5, we can nest 4 Any messages
1771+
# (depth 1 = outer Any, depth 2-4 = nested Anys, depth 5 = final value)
1772+
nested_any = {
1773+
'@type': 'type.googleapis.com/google.protobuf.Any',
1774+
'value': {
1775+
'@type': 'type.googleapis.com/google.protobuf.Any',
1776+
'value': {
1777+
'@type': 'type.googleapis.com/google.protobuf.Any',
1778+
'value': {
1779+
'@type': 'type.googleapis.com/google.protobuf.Any',
1780+
'value': {
1781+
'@type': 'type.googleapis.com/google.protobuf.Any',
1782+
'value': {},
1783+
},
1784+
},
1785+
},
1786+
},
1787+
}
1788+
1789+
# Should raise ParseError due to exceeding max depth, not RecursionError
1790+
self.assertRaisesRegex(
1791+
json_format.ParseError,
1792+
'Message too deep. Max recursion depth is 5',
1793+
json_format.ParseDict,
1794+
nested_any,
1795+
message,
1796+
max_recursion_depth=5,
1797+
)
1798+
1799+
# Verify that Any messages within the limit can be parsed successfully
1800+
# With max_recursion_depth=5, we can nest up to 4 Any messages
1801+
shallow_any = {
1802+
'@type': 'type.googleapis.com/google.protobuf.Any',
1803+
'value': {
1804+
'@type': 'type.googleapis.com/google.protobuf.Any',
1805+
'value': {
1806+
'@type': 'type.googleapis.com/google.protobuf.Any',
1807+
'value': {
1808+
'@type': 'type.googleapis.com/google.protobuf.Any',
1809+
'value': {},
1810+
},
1811+
},
1812+
},
1813+
}
1814+
json_format.ParseDict(shallow_any, message, max_recursion_depth=5)
1815+
1816+
def testAnyRecursionDepthBoundary(self):
1817+
"""Test recursion depth boundary behavior (exclusive upper limit)."""
1818+
message = any_pb2.Any()
1819+
1820+
# Create nested Any at depth exactly 4 (should succeed with max_recursion_depth=5)
1821+
depth_4_any = {
1822+
'@type': 'type.googleapis.com/google.protobuf.Any',
1823+
'value': {
1824+
'@type': 'type.googleapis.com/google.protobuf.Any',
1825+
'value': {
1826+
'@type': 'type.googleapis.com/google.protobuf.Any',
1827+
'value': {
1828+
'@type': 'type.googleapis.com/google.protobuf.Any',
1829+
'value': {},
1830+
},
1831+
},
1832+
},
1833+
}
1834+
# This should succeed: depth 4 < max_recursion_depth 5
1835+
json_format.ParseDict(depth_4_any, message, max_recursion_depth=5)
1836+
1837+
# Create nested Any at depth exactly 5 (should fail with max_recursion_depth=5)
1838+
depth_5_any = {
1839+
'@type': 'type.googleapis.com/google.protobuf.Any',
1840+
'value': {
1841+
'@type': 'type.googleapis.com/google.protobuf.Any',
1842+
'value': {
1843+
'@type': 'type.googleapis.com/google.protobuf.Any',
1844+
'value': {
1845+
'@type': 'type.googleapis.com/google.protobuf.Any',
1846+
'value': {
1847+
'@type': 'type.googleapis.com/google.protobuf.Any',
1848+
'value': {},
1849+
},
1850+
},
1851+
},
1852+
},
1853+
}
1854+
# This should fail: depth 5 == max_recursion_depth 5 (exclusive limit)
1855+
self.assertRaisesRegex(
1856+
json_format.ParseError,
1857+
'Message too deep. Max recursion depth is 5',
1858+
json_format.ParseDict,
1859+
depth_5_any,
1860+
message,
1861+
max_recursion_depth=5,
1862+
)
1863+
17631864
def testJsonNameConflictSerilize(self):
17641865
message = more_messages_pb2.ConflictJsonName(value=2)
17651866
self.assertEqual(

python/google/protobuf/json_format.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,10 @@ def ConvertMessage(self, value, message, path):
506506
Raises:
507507
ParseError: In case of convert problems.
508508
"""
509+
# Increment recursion depth at message entry. The max_recursion_depth limit
510+
# is exclusive: a depth value equal to max_recursion_depth will trigger an
511+
# error. For example, with max_recursion_depth=5, nesting up to depth 4 is
512+
# allowed, but attempting depth 5 raises ParseError.
509513
self.recursion_depth += 1
510514
if self.recursion_depth > self.max_recursion_depth:
511515
raise ParseError(
@@ -726,12 +730,11 @@ def _ConvertAnyMessage(self, value, message, path):
726730
value['value'], sub_message, '{0}.value'.format(path)
727731
)
728732
elif full_name in _WKTJSONMETHODS:
729-
methodcaller(
730-
_WKTJSONMETHODS[full_name][1],
731-
value['value'],
732-
sub_message,
733-
'{0}.value'.format(path),
734-
)(self)
733+
# For well-known types (including nested Any), use ConvertMessage
734+
# to ensure recursion depth is properly tracked
735+
self.ConvertMessage(
736+
value['value'], sub_message, '{0}.value'.format(path)
737+
)
735738
else:
736739
del value['@type']
737740
try:

0 commit comments

Comments
 (0)