From 452675f701d6b3f8d3bd7cb955bdc156c93e72be Mon Sep 17 00:00:00 2001 From: Keith Bennett Date: Thu, 29 Mar 2018 16:45:11 -0500 Subject: [PATCH] use RAII type class to guard against stack depth recursion instead of error-prone manual increment/check/decrement --- include/yaml-cpp/depthguard.h | 74 +++++++++++++++++++++++++++++++++++ src/depthguard.cpp | 14 +++++++ src/singledocparser.cpp | 18 ++------- src/singledocparser.h | 4 +- 4 files changed, 94 insertions(+), 16 deletions(-) create mode 100644 include/yaml-cpp/depthguard.h create mode 100644 src/depthguard.cpp diff --git a/include/yaml-cpp/depthguard.h b/include/yaml-cpp/depthguard.h new file mode 100644 index 0000000..6aac81a --- /dev/null +++ b/include/yaml-cpp/depthguard.h @@ -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 +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 diff --git a/src/depthguard.cpp b/src/depthguard.cpp new file mode 100644 index 0000000..6d47eba --- /dev/null +++ b/src/depthguard.cpp @@ -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 diff --git a/src/singledocparser.cpp b/src/singledocparser.cpp index 50d0c40..a433469 100644 --- a/src/singledocparser.cpp +++ b/src/singledocparser.cpp @@ -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,9 +48,8 @@ void SingleDocParser::HandleDocument(EventHandler& eventHandler) { } void SingleDocParser::HandleNode(EventHandler& eventHandler) { - if (depth > depth_limit) { - throw ParserException(m_scanner.mark(), ErrorMsg::BAD_FILE); - } + DepthGuard depthguard(depth, m_scanner.mark(), ErrorMsg::BAD_FILE); + // an empty node *is* a possibility if (m_scanner.empty()) { 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 if (m_scanner.peek().type == Token::VALUE) { - depth++; eventHandler.OnMapStart(mark, "?", NullAnchor, EmitterStyle::Default); HandleMap(eventHandler); eventHandler.OnMapEnd(); - depth--; return; } @@ -110,42 +108,32 @@ void SingleDocParser::HandleNode(EventHandler& eventHandler) { m_scanner.pop(); return; case Token::FLOW_SEQ_START: - depth++; eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Flow); HandleSequence(eventHandler); eventHandler.OnSequenceEnd(); - depth--; return; case Token::BLOCK_SEQ_START: - depth++; eventHandler.OnSequenceStart(mark, tag, anchor, EmitterStyle::Block); HandleSequence(eventHandler); eventHandler.OnSequenceEnd(); - depth--; return; case Token::FLOW_MAP_START: - depth++; eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow); HandleMap(eventHandler); eventHandler.OnMapEnd(); - depth--; return; case Token::BLOCK_MAP_START: - depth++; eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Block); HandleMap(eventHandler); eventHandler.OnMapEnd(); - depth--; return; case Token::KEY: // compact maps can only go in a flow sequence if (m_pCollectionStack->GetCurCollectionType() == CollectionType::FlowSeq) { - depth++; eventHandler.OnMapStart(mark, tag, anchor, EmitterStyle::Flow); HandleMap(eventHandler); eventHandler.OnMapEnd(); - depth--; return; } break; diff --git a/src/singledocparser.h b/src/singledocparser.h index 47f9b94..2e1f303 100644 --- a/src/singledocparser.h +++ b/src/singledocparser.h @@ -15,6 +15,8 @@ namespace YAML { class CollectionStack; +template class DepthGuard; // depthguard.h +class DeepRecursion; // an exception which may be thrown from excessive call stack recursion, see depthguard.h class EventHandler; class Node; class Scanner; @@ -55,8 +57,8 @@ class SingleDocParser { anchor_t LookupAnchor(const Mark& mark, const std::string& name) const; private: + using DepthGuard = YAML::DepthGuard<2000>; int depth = 0; - int depth_limit = 2000; Scanner& m_scanner; const Directives& m_directives; std::unique_ptr m_pCollectionStack;