From 56c9afa7c8841641d7f5719eeb2e275ebfed8657 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 11 May 2021 22:26:15 -0700 Subject: [PATCH] XPath: Improve recursion limit for deep chains of // Since foo//bar//baz adds two nodes for each //, we need to increment the depth by 2 on each iteration to limit the AST correctly. Fixes the stack overflow found by cluster-fuzz (I suspect the issue there is a bit deeper, but this part is definitely a bug and as such I'd rather wait for the next test case for now). --- src/pugixml.cpp | 8 +++++--- tests/test_xpath_parse.cpp | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pugixml.cpp b/src/pugixml.cpp index 0d36e3b..13b9504 100644 --- a/src/pugixml.cpp +++ b/src/pugixml.cpp @@ -11822,15 +11822,17 @@ PUGI__NS_BEGIN lexeme_t l = _lexer.current(); _lexer.next(); - if (++_depth > xpath_ast_depth_limit) - return error_rec(); - if (l == lex_double_slash) { n = alloc_node(ast_step, n, axis_descendant_or_self, nodetest_type_node, 0); if (!n) return 0; + + ++_depth; } + if (++_depth > xpath_ast_depth_limit) + return error_rec(); + n = parse_step(n); if (!n) return 0; } diff --git a/tests/test_xpath_parse.cpp b/tests/test_xpath_parse.cpp index 494b0ae..bfa59ff 100644 --- a/tests/test_xpath_parse.cpp +++ b/tests/test_xpath_parse.cpp @@ -402,6 +402,7 @@ TEST(xpath_parse_depth_limit) CHECK_XPATH_FAIL((STR("/foo") + rep(STR("/x"), limit)).c_str()); CHECK_XPATH_FAIL((STR("1") + rep(STR("+1"), limit)).c_str()); CHECK_XPATH_FAIL((STR("concat(") + rep(STR("1,"), limit) + STR("1)")).c_str()); + CHECK_XPATH_FAIL((STR("/foo") + rep(STR("//x"), limit / 2)).c_str()); } TEST_XML(xpath_parse_location_path, "")