From 748443a9cd23b87e3ae85273bb341d07592765a5 Mon Sep 17 00:00:00 2001 From: Richard Shaw Date: Sun, 16 Sep 2018 07:20:56 -0500 Subject: [PATCH] Add patch for CVE-2017-5950. --- CVE-2017-5950.patch | 383 ++++++++++++++++++++++++++++++++++++++++++++ yaml-cpp.spec | 9 +- 2 files changed, 388 insertions(+), 4 deletions(-) create mode 100644 CVE-2017-5950.patch diff --git a/CVE-2017-5950.patch b/CVE-2017-5950.patch new file mode 100644 index 0000000..3a96432 --- /dev/null +++ b/CVE-2017-5950.patch @@ -0,0 +1,383 @@ +From d540476e31b080aa1f903ad20ec0426dd3838be7 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= +Date: Tue, 25 Apr 2017 20:10:20 -0400 +Subject: [PATCH 1/4] fix stack overflow in HandleNode() (CVE-2017-5950) + +simply set a hardcoded recursion limit to 2000 (inspired by Python's) +to avoid infinitely recursing into arbitrary data structures + +assert() the depth. unsure if this is the right approach, but given +that HandleNode() is "void", I am not sure how else to return an +error. the problem with this approach of course is that it will still +crash the caller, unless they have proper exception handling in place. + +Closes: #459 +--- + src/singledocparser.cpp | 2 ++ + src/singledocparser.h | 2 ++ + 2 files changed, 4 insertions(+) + +diff --git a/src/singledocparser.cpp b/src/singledocparser.cpp +index a27c1c3b..1b4262ee 100644 +--- a/src/singledocparser.cpp ++++ b/src/singledocparser.cpp +@@ -46,6 +46,8 @@ void SingleDocParser::HandleDocument(EventHandler& eventHandler) { + } + + void SingleDocParser::HandleNode(EventHandler& eventHandler) { ++ assert(depth < depth_limit); ++ depth++; + // an empty node *is* a possibility + if (m_scanner.empty()) { + eventHandler.OnNull(m_scanner.mark(), NullAnchor); +diff --git a/src/singledocparser.h b/src/singledocparser.h +index 2b92067c..7046f1e2 100644 +--- a/src/singledocparser.h ++++ b/src/singledocparser.h +@@ -51,6 +51,8 @@ class SingleDocParser : private noncopyable { + anchor_t LookupAnchor(const Mark& mark, const std::string& name) const; + + private: ++ int depth = 0; ++ int depth_limit = 2000; + Scanner& m_scanner; + const Directives& m_directives; + std::unique_ptr m_pCollectionStack; + +From ac00ef937702598aa27739c8c46be37ac5699039 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= +Date: Wed, 26 Apr 2017 10:25:43 -0400 +Subject: [PATCH 2/4] throw an exception instead of using assert() + +assert() may be compiled out in production and is clunkier to catch. + +some ParserException are already thrown elsewhere in the code and it +seems to make sense to reuse the primitive, although it may still +crash improperly configured library consumers, those who do not handle +exceptions explicitly. + +we use the BAD_FILE error message because at this point we do not +exactly know which specific data structure led to the recursion. +--- + src/singledocparser.cpp | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/singledocparser.cpp b/src/singledocparser.cpp +index 1b4262ee..1af13f49 100644 +--- a/src/singledocparser.cpp ++++ b/src/singledocparser.cpp +@@ -46,7 +46,9 @@ void SingleDocParser::HandleDocument(EventHandler& eventHandler) { + } + + void SingleDocParser::HandleNode(EventHandler& eventHandler) { +- assert(depth < depth_limit); ++ if (depth > depth_limit) { ++ throw ParserException(m_scanner.mark(), ErrorMsg::BAD_FILE); ++ } + depth++; + // an empty node *is* a possibility + if (m_scanner.empty()) { + +From e78e3bf6a6d61ca321af90d213dc4435ed5cf602 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= +Date: Wed, 26 Apr 2017 10:39:45 -0400 +Subject: [PATCH 3/4] increase and decrease depth properly on subhandlers + +the original implementation couldn't parse a document with more than +depth_limit entries. now we explicitly increase *and* decrease the +depth on specific handlers like maps, sequences and so on - any +handler that may in turn callback into HandleNode(). + +this is a little clunky - I would have prefered to increment and +decrement the counter in only one place, but there are many different +return points and this is not Golang so I can't think of a better way +to to this. +--- + src/singledocparser.cpp | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/src/singledocparser.cpp b/src/singledocparser.cpp +index 1af13f49..89234867 100644 +--- a/src/singledocparser.cpp ++++ b/src/singledocparser.cpp +@@ -49,7 +49,6 @@ void SingleDocParser::HandleNode(EventHandler& eventHandler) { + if (depth > depth_limit) { + throw ParserException(m_scanner.mark(), ErrorMsg::BAD_FILE); + } +- depth++; + // an empty node *is* a possibility + if (m_scanner.empty()) { + eventHandler.OnNull(m_scanner.mark(), NullAnchor); +@@ -61,9 +60,11 @@ 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; + } + +@@ -98,32 +99,42 @@ 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; + +From 1690cacb3ff6d927286ded92b8fedd37b4045c7c Mon Sep 17 00:00:00 2001 +From: Keith Bennett +Date: Thu, 29 Mar 2018 16:45:11 -0500 +Subject: [PATCH 4/4] 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 00000000..6aac81aa +--- /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 00000000..6d47eba3 +--- /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 89234867..37cc1f51 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 +@@ -46,9 +47,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); +@@ -60,11 +60,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; + } + +@@ -99,42 +97,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 7046f1e2..f1676c43 100644 +--- a/src/singledocparser.h ++++ b/src/singledocparser.h +@@ -16,6 +16,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; +@@ -51,8 +53,8 @@ class SingleDocParser : private noncopyable { + 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; diff --git a/yaml-cpp.spec b/yaml-cpp.spec index e902487..d8bcb69 100644 --- a/yaml-cpp.spec +++ b/yaml-cpp.spec @@ -2,15 +2,15 @@ Name: yaml-cpp Version: 0.6.1 -Release: 3%{?dist} +Release: 4%{?dist} Summary: A YAML parser and emitter for C++ -Group: Development/Libraries License: MIT URL: https://github.com/jbeder/yaml-cpp Source0: https://github.com/jbeder/yaml-cpp/archive/%{name}-%{version}.tar.gz Patch0: yaml-cpp-static.patch Patch1: yaml-cpp-include_dir.patch +Patch2: CVE-2017-5950.patch BuildRequires: cmake gcc gcc-c++ @@ -20,7 +20,6 @@ yaml-cpp is a YAML parser and emitter in C++ written around the YAML 1.2 spec. %package devel Summary: Development files for %{name} -Group: Development/Libraries License: MIT Requires: %{name}%{?_isa} = %{version}-%{release} Requires: pkgconfig @@ -33,7 +32,6 @@ developing applications that use %{name}. %package static Summary: Static library for %{name} -Group: Development/Libraries License: MIT Requires: %{name}-devel%{?_isa} = %{version}-%{release} @@ -97,6 +95,9 @@ pushd build_static %changelog +* Sun Sep 16 2018 Richard Shaw - 0.6.1-4 +- Add patch for CVE-2017-5950. + * Sat Jul 14 2018 Fedora Release Engineering - 0.6.1-3 - Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild