use RAII type class to guard against stack depth recursion instead of error-prone manual increment/check/decrement

This commit is contained in:
Keith Bennett 2018-03-29 16:45:11 -05:00 committed by Alan Griffiths
parent 6c5081b364
commit 452675f701
4 changed files with 94 additions and 16 deletions

View File

@ -0,0 +1,74 @@
#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 {
int m_atDepth = 0;
public:
// no custom dtor needed, but virtual dtor necessary to prevent slicing
virtual ~DeepRecursion() = default;
// construct an exception explaining how deep you were
DeepRecursion(int at_depth, const Mark& mark_, const std::string& msg_);
// query how deep you were when the exception was thrown
int AtDepth() const;
};
/**
* @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 /* final because non-virtual dtor */ {
int & m_depth;
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 is neither copyable nor moveable.
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;
}
};
} // namespace YAML
#endif // DEPTH_GUARD_H_00000000000000000000000000000000000000000000000000000000

14
src/depthguard.cpp Normal file
View File

@ -0,0 +1,14 @@
#include "yaml-cpp/depthguard.h"
namespace YAML {
DeepRecursion::DeepRecursion(int at_depth, const Mark& mark_, const std::string& msg_)
: ParserException(mark_, msg_),
m_atDepth(at_depth) {
}
int DeepRecursion::AtDepth() const {
return m_atDepth;
}
} // namespace YAML

View File

@ -7,6 +7,7 @@
#include "singledocparser.h" #include "singledocparser.h"
#include "tag.h" #include "tag.h"
#include "token.h" #include "token.h"
#include "yaml-cpp/depthguard.h"
#include "yaml-cpp/emitterstyle.h" #include "yaml-cpp/emitterstyle.h"
#include "yaml-cpp/eventhandler.h" #include "yaml-cpp/eventhandler.h"
#include "yaml-cpp/exceptions.h" // IWYU pragma: keep #include "yaml-cpp/exceptions.h" // IWYU pragma: keep
@ -47,9 +48,8 @@ void SingleDocParser::HandleDocument(EventHandler& eventHandler) {
} }
void SingleDocParser::HandleNode(EventHandler& eventHandler) { void SingleDocParser::HandleNode(EventHandler& eventHandler) {
if (depth > depth_limit) { DepthGuard depthguard(depth, m_scanner.mark(), ErrorMsg::BAD_FILE);
throw ParserException(m_scanner.mark(), ErrorMsg::BAD_FILE);
}
// an empty node *is* a possibility // an empty node *is* a possibility
if (m_scanner.empty()) { if (m_scanner.empty()) {
eventHandler.OnNull(m_scanner.mark(), NullAnchor); eventHandler.OnNull(m_scanner.mark(), NullAnchor);
@ -61,11 +61,9 @@ void SingleDocParser::HandleNode(EventHandler& eventHandler) {
// special case: a value node by itself must be a map, with no header // special case: a value node by itself must be a map, with no header
if (m_scanner.peek().type == Token::VALUE) { if (m_scanner.peek().type == Token::VALUE) {
depth++;
eventHandler.OnMapStart(mark, "?", NullAnchor, EmitterStyle::Default); eventHandler.OnMapStart(mark, "?", NullAnchor, EmitterStyle::Default);
HandleMap(eventHandler); HandleMap(eventHandler);
eventHandler.OnMapEnd(); eventHandler.OnMapEnd();
depth--;
return; return;
} }
@ -110,42 +108,32 @@ void SingleDocParser::HandleNode(EventHandler& eventHandler) {
m_scanner.pop(); m_scanner.pop();
return; return;
case Token::FLOW_SEQ_START: case Token::FLOW_SEQ_START:
depth++;
eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Flow); eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Flow);
HandleSequence(eventHandler); HandleSequence(eventHandler);
eventHandler.OnSequenceEnd(); eventHandler.OnSequenceEnd();
depth--;
return; return;
case Token::BLOCK_SEQ_START: case Token::BLOCK_SEQ_START:
depth++;
eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Block); eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Block);
HandleSequence(eventHandler); HandleSequence(eventHandler);
eventHandler.OnSequenceEnd(); eventHandler.OnSequenceEnd();
depth--;
return; return;
case Token::FLOW_MAP_START: case Token::FLOW_MAP_START:
depth++;
eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow); eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow);
HandleMap(eventHandler); HandleMap(eventHandler);
eventHandler.OnMapEnd(); eventHandler.OnMapEnd();
depth--;
return; return;
case Token::BLOCK_MAP_START: case Token::BLOCK_MAP_START:
depth++;
eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Block); eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Block);
HandleMap(eventHandler); HandleMap(eventHandler);
eventHandler.OnMapEnd(); eventHandler.OnMapEnd();
depth--;
return; return;
case Token::KEY: case Token::KEY:
// compact maps can only go in a flow sequence // compact maps can only go in a flow sequence
if (m_pCollectionStack->GetCurCollectionType() == if (m_pCollectionStack->GetCurCollectionType() ==
CollectionType::FlowSeq) { CollectionType::FlowSeq) {
depth++;
eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow); eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow);
HandleMap(eventHandler); HandleMap(eventHandler);
eventHandler.OnMapEnd(); eventHandler.OnMapEnd();
depth--;
return; return;
} }
break; break;

View File

@ -15,6 +15,8 @@
namespace YAML { namespace YAML {
class CollectionStack; class CollectionStack;
template <int> class DepthGuard; // depthguard.h
class DeepRecursion; // an exception which may be thrown from excessive call stack recursion, see depthguard.h
class EventHandler; class EventHandler;
class Node; class Node;
class Scanner; class Scanner;
@ -55,8 +57,8 @@ class SingleDocParser {
anchor_t LookupAnchor(const Mark& mark, const std::string& name) const; anchor_t LookupAnchor(const Mark& mark, const std::string& name) const;
private: private:
using DepthGuard = YAML::DepthGuard<2000>;
int depth = 0; int depth = 0;
int depth_limit = 2000;
Scanner& m_scanner; Scanner& m_scanner;
const Directives& m_directives; const Directives& m_directives;
std::unique_ptr<CollectionStack> m_pCollectionStack; std::unique_ptr<CollectionStack> m_pCollectionStack;