From 4d9a633ed1595966dc59bb35ac8db9e927c654cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= Date: Tue, 16 Jul 2024 12:25:17 +0200 Subject: [PATCH] Fix invalid memory uses in libdnf::ConfigParser::substitute_expression() Resolves: RHEL-36777 --- ...figParser-fix-use-out-of-scope-leaks.patch | 128 ++++++++++++++++++ ...s-for-shell-style-variable-expansion.patch | 42 ++++++ libdnf.spec | 7 +- 3 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 0005-ConfigParser-fix-use-out-of-scope-leaks.patch create mode 100644 0006-Add-tests-for-shell-style-variable-expansion.patch diff --git a/0005-ConfigParser-fix-use-out-of-scope-leaks.patch b/0005-ConfigParser-fix-use-out-of-scope-leaks.patch new file mode 100644 index 0000000..636d757 --- /dev/null +++ b/0005-ConfigParser-fix-use-out-of-scope-leaks.patch @@ -0,0 +1,128 @@ +From 1f6c2479260e2d26a861b871c443a5b960523a71 Mon Sep 17 00:00:00 2001 +From: Evan Goode +Date: Tue, 7 May 2024 16:33:03 +0000 +Subject: [PATCH] ConfigParser: fix use-out-of-scope leaks +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Petr Písař +--- + libdnf/conf/ConfigParser.cpp | 48 ++++++++++++++++++++++++------------ + 1 file changed, 32 insertions(+), 16 deletions(-) + +diff --git a/libdnf/conf/ConfigParser.cpp b/libdnf/conf/ConfigParser.cpp +index 76e7f9cc..977da757 100644 +--- a/libdnf/conf/ConfigParser.cpp ++++ b/libdnf/conf/ConfigParser.cpp +@@ -95,7 +95,9 @@ std::pair ConfigParser::substitute_expression(const std::st + const auto & variable_key = res.substr(pos_variable, pos_after_variable - pos_variable); + const auto variable_mapping = substitutions.find(variable_key); + +- const std::string * variable_value = nullptr; ++ // No std::optional here. ++ bool variable_value_has_value{false}; ++ std::string variable_value{""}; + + if (variable_mapping == substitutions.end()) { + if (variable_key == "releasever_major" || variable_key == "releasever_minor") { +@@ -103,17 +105,22 @@ std::pair ConfigParser::substitute_expression(const std::st + if (releasever_mapping != substitutions.end()) { + const auto & releasever_split = ConfigParser::split_releasever(releasever_mapping->second); + if (variable_key == "releasever_major") { +- variable_value = &std::get<0>(releasever_split); ++ variable_value = std::get<0>(releasever_split); ++ variable_value_has_value = true; + } else { +- variable_value = &std::get<1>(releasever_split); ++ variable_value = std::get<1>(releasever_split); ++ variable_value_has_value = true; + } + } + } + } else { +- variable_value = &variable_mapping->second; ++ variable_value = variable_mapping->second; ++ variable_value_has_value = true; + } + +- const std::string * subst_str = nullptr; ++ // No std::optional here ++ std::string subst_str{""}; ++ bool subst_str_has_value{false}; + + size_t pos_after_variable_expression; + +@@ -153,20 +160,23 @@ std::pair ConfigParser::substitute_expression(const std::st + // If variable is unset or empty, the expansion of word is + // substituted. Otherwise, the value of variable is + // substituted. +- if (variable_value == nullptr || variable_value->empty()) { +- subst_str = &expanded_word; ++ if (!variable_value_has_value || variable_value.empty()) { ++ subst_str = expanded_word; ++ subst_str_has_value = true; + } else { + subst_str = variable_value; ++ subst_str_has_value = true; + } + } else if (expansion_mode == '+') { + // ${variable:+word} (alternate value) + // If variable is unset or empty nothing is substituted. + // Otherwise, the expansion of word is substituted. +- if (variable_value == nullptr || variable_value->empty()) { +- const std::string empty{}; +- subst_str = ∅ ++ if (!variable_value_has_value || variable_value.empty()) { ++ subst_str = ""; ++ subst_str_has_value = true; + } else { +- subst_str = &expanded_word; ++ subst_str = expanded_word; ++ subst_str_has_value = true; + } + } else { + // Unknown expansion mode, continue after the ':' +@@ -176,7 +186,10 @@ std::pair ConfigParser::substitute_expression(const std::st + pos_after_variable_expression = pos_after_word + 1; + } else if (res[pos_after_variable] == '}') { + // ${variable} +- subst_str = variable_value; ++ if (variable_value_has_value) { ++ subst_str = variable_value; ++ subst_str_has_value = true; ++ } + // Move past the closing '}' + pos_after_variable_expression = pos_after_variable + 1; + } else { +@@ -186,20 +199,23 @@ std::pair ConfigParser::substitute_expression(const std::st + } + } else { + // No braces, we have a $variable +- subst_str = variable_value; ++ if (variable_value_has_value) { ++ subst_str = variable_value; ++ subst_str_has_value = true; ++ } + pos_after_variable_expression = pos_after_variable; + } + + // If there is no substitution to make, move past the variable expression and continue. +- if (subst_str == nullptr) { ++ if (!subst_str_has_value) { + total_scanned += pos_after_variable_expression - pos; + pos = pos_after_variable_expression; + continue; + } + +- res.replace(pos, pos_after_variable_expression - pos, *subst_str); ++ res.replace(pos, pos_after_variable_expression - pos, subst_str); + total_scanned += pos_after_variable_expression - pos; +- pos += subst_str->length(); ++ pos += subst_str.length(); + } else { + total_scanned += 1; + pos += 1; +-- +2.45.2 + diff --git a/0006-Add-tests-for-shell-style-variable-expansion.patch b/0006-Add-tests-for-shell-style-variable-expansion.patch new file mode 100644 index 0000000..7929439 --- /dev/null +++ b/0006-Add-tests-for-shell-style-variable-expansion.patch @@ -0,0 +1,42 @@ +From 18f49a49be14f80233613710e84dda47c5958252 Mon Sep 17 00:00:00 2001 +From: Evan Goode +Date: Tue, 7 May 2024 16:28:59 +0000 +Subject: [PATCH] Add tests for shell-style variable expansion +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Petr Písař +--- + tests/libdnf/conf/ConfigParserTest.cpp | 17 +++++++++++++++++ + 1 file changed, 17 insertions(+) + +diff --git a/tests/libdnf/conf/ConfigParserTest.cpp b/tests/libdnf/conf/ConfigParserTest.cpp +index 70278196..1448d8d3 100644 +--- a/tests/libdnf/conf/ConfigParserTest.cpp ++++ b/tests/libdnf/conf/ConfigParserTest.cpp +@@ -30,4 +30,21 @@ void ConfigParserTest::testConfigParserReleasever() + libdnf::ConfigParser::substitute(text, substitutions); + CPPUNIT_ASSERT(text == "major: , minor: "); + } ++ { ++ std::map substitutions = { ++ {"var1", "value123"}, ++ {"var2", "456"}, ++ }; ++ std::string text = "foo$var1-bar"; ++ libdnf::ConfigParser::substitute(text, substitutions); ++ CPPUNIT_ASSERT(text == "foovalue123-bar"); ++ ++ text = "${var1:+alternate}-${unset:-default}-${nn:+n${nn:-${nnn:}"; ++ libdnf::ConfigParser::substitute(text, substitutions); ++ CPPUNIT_ASSERT(text == "alternate-default-${nn:+n${nn:-${nnn:}"); ++ ++ text = "${unset:-${var1:+${var2:+$var2}}}"; ++ libdnf::ConfigParser::substitute(text, substitutions); ++ CPPUNIT_ASSERT(text == "456"); ++ } + } +-- +2.45.2 + diff --git a/libdnf.spec b/libdnf.spec index 1613440..f21dde7 100644 --- a/libdnf.spec +++ b/libdnf.spec @@ -56,7 +56,7 @@ Name: libdnf Version: %{libdnf_major_version}.%{libdnf_minor_version}.%{libdnf_micro_version} -Release: 4%{?dist} +Release: 5%{?dist} Summary: Library providing simplified C and Python API to libsolv License: LGPL-2.1-or-later URL: https://github.com/rpm-software-management/libdnf @@ -65,6 +65,8 @@ Patch1: 0001-context-use-rpmtsAddReinstallElement-when-doing-a-re.patch Patch2: 0002-Since-we-use-rpmtsAddReinstallElement-rpm-also-unins.patch Patch3: 0003-MergedTransaction-Fix-invalid-memory-access-when-dro.patch Patch4: 0004-Set-pool-flag-to-fix-pool_addfileprovides_queue-with.patch +Patch5: 0005-ConfigParser-fix-use-out-of-scope-leaks.patch +Patch6: 0006-Add-tests-for-shell-style-variable-expansion.patch BuildRequires: cmake BuildRequires: gcc @@ -308,6 +310,9 @@ popd %endif %changelog +* Tue Jul 16 2024 Petr Pisar - 0.73.1-5 +- Fix invalid memory uses in libdnf::ConfigParser::substitute_expression() (RHEL-36777) + * Tue Jul 02 2024 Petr Pisar - 0.73.1-4 - Fix sack initialization to enable postponed addition of filelists (RHEL-12355)