Fix stack overflow (#807)

Fix stack overflow in HandleNode by explicitly limiting the depth of recursion.
This commit is contained in:
Alan Griffiths
2020-04-09 19:02:10 +01:00
committed by GitHub
parent 70205f5d88
commit 4edff1fa5d
5 changed files with 140 additions and 0 deletions

View File

@@ -0,0 +1,77 @@
#ifndef DEPTH_GUARD_H_00000000000000000000000000000000000000000000000000000000
#define DEPTH_GUARD_H_00000000000000000000000000000000000000000000000000000000
#if defined(_MSC_VER) || \
(defined(__GNUC__) && (__GNUC__ == 3 && __GNUC_MINOR__ >= 4) || \
(__GNUC__ >= 4)) // GCC supports "pragma once" correctly since 3.4
#pragma once
#endif
#include "exceptions.h"
namespace YAML {
/**
* @brief The DeepRecursion class
* An exception class which is thrown by DepthGuard. Ideally it should be
* a member of DepthGuard. However, DepthGuard is a templated class which means
* that any catch points would then need to know the template parameters. It is
* simpler for clients to not have to know at the catch point what was the
* maximum depth.
*/
class DeepRecursion : public ParserException {
public:
virtual ~DeepRecursion() = default;
DeepRecursion(int depth, const Mark& mark_, const std::string& msg_);
// Returns the recursion depth when the exception was thrown
int depth() const {
return m_depth;
}
private:
int m_depth = 0;
};
/**
* @brief The DepthGuard class
* DepthGuard takes a reference to an integer. It increments the integer upon
* construction of DepthGuard and decrements the integer upon destruction.
*
* If the integer would be incremented past max_depth, then an exception is
* thrown. This is ideally geared toward guarding against deep recursion.
*
* @param max_depth
* compile-time configurable maximum depth.
*/
template <int max_depth = 2000>
class DepthGuard final {
public:
DepthGuard(int & depth_, const Mark& mark_, const std::string& msg_) : m_depth(depth_) {
++m_depth;
if ( max_depth <= m_depth ) {
throw DeepRecursion{m_depth, mark_, msg_};
}
}
DepthGuard(const DepthGuard & copy_ctor) = delete;
DepthGuard(DepthGuard && move_ctor) = delete;
DepthGuard & operator=(const DepthGuard & copy_assign) = delete;
DepthGuard & operator=(DepthGuard && move_assign) = delete;
~DepthGuard() {
--m_depth;
}
int current_depth() const {
return m_depth;
}
private:
int & m_depth;
};
} // namespace YAML
#endif // DEPTH_GUARD_H_00000000000000000000000000000000000000000000000000000000

10
src/depthguard.cpp Normal file
View File

@@ -0,0 +1,10 @@
#include "yaml-cpp/depthguard.h"
namespace YAML {
DeepRecursion::DeepRecursion(int depth, const Mark& mark_, const std::string& msg_)
: ParserException(mark_, msg_),
m_depth(depth) {
}
} // namespace YAML

View File

@@ -7,6 +7,7 @@
#include "singledocparser.h"
#include "tag.h"
#include "token.h"
#include "yaml-cpp/depthguard.h"
#include "yaml-cpp/emitterstyle.h"
#include "yaml-cpp/eventhandler.h"
#include "yaml-cpp/exceptions.h" // IWYU pragma: keep
@@ -47,6 +48,8 @@ void SingleDocParser::HandleDocument(EventHandler& eventHandler) {
}
void SingleDocParser::HandleNode(EventHandler& eventHandler) {
DepthGuard<2000> depthguard(depth, m_scanner.mark(), ErrorMsg::BAD_FILE);
// an empty node *is* a possibility
if (m_scanner.empty()) {
eventHandler.OnNull(m_scanner.mark(), NullAnchor);

View File

@@ -15,6 +15,7 @@
namespace YAML {
class CollectionStack;
template <int> class DepthGuard; // depthguard.h
class EventHandler;
class Node;
class Scanner;
@@ -55,6 +56,7 @@ class SingleDocParser {
anchor_t LookupAnchor(const Mark& mark, const std::string& name) const;
private:
int depth = 0;
Scanner& m_scanner;
const Directives& m_directives;
std::unique_ptr<CollectionStack> m_pCollectionStack;

View File

@@ -1,9 +1,12 @@
#include <yaml-cpp/depthguard.h>
#include "yaml-cpp/parser.h"
#include "yaml-cpp/exceptions.h"
#include "mock_event_handler.h"
#include "gtest/gtest.h"
using YAML::Parser;
using YAML::MockEventHandler;
using ::testing::NiceMock;
using ::testing::StrictMock;
TEST(ParserTest, Empty) {
@@ -14,3 +17,48 @@ TEST(ParserTest, Empty) {
StrictMock<MockEventHandler> handler;
EXPECT_FALSE(parser.HandleNextDocument(handler));
}
TEST(ParserTest, CVE_2017_5950) {
std::string excessive_recursion;
for (auto i = 0; i != 16384; ++i)
excessive_recursion.push_back('[');
std::istringstream input{excessive_recursion};
Parser parser{input};
NiceMock<MockEventHandler> handler;
EXPECT_THROW(parser.HandleNextDocument(handler), YAML::DeepRecursion);
}
TEST(ParserTest, CVE_2018_20573) {
std::string excessive_recursion;
for (auto i = 0; i != 20535; ++i)
excessive_recursion.push_back('{');
std::istringstream input{excessive_recursion};
Parser parser{input};
NiceMock<MockEventHandler> handler;
EXPECT_THROW(parser.HandleNextDocument(handler), YAML::DeepRecursion);
}
TEST(ParserTest, CVE_2018_20574) {
std::string excessive_recursion;
for (auto i = 0; i != 21989; ++i)
excessive_recursion.push_back('{');
std::istringstream input{excessive_recursion};
Parser parser{input};
NiceMock<MockEventHandler> handler;
EXPECT_THROW(parser.HandleNextDocument(handler), YAML::DeepRecursion);
}
TEST(ParserTest, CVE_2019_6285) {
std::string excessive_recursion;
for (auto i = 0; i != 23100; ++i)
excessive_recursion.push_back('[');
excessive_recursion.push_back('f');
std::istringstream input{excessive_recursion};
Parser parser{input};
NiceMock<MockEventHandler> handler;
EXPECT_THROW(parser.HandleNextDocument(handler), YAML::DeepRecursion);
}