From 3ec8d6521182b7f76897959694d70283b13ace14 Mon Sep 17 00:00:00 2001
From: Guillem Jover <guillem@debian.org>
Date: Tue, 28 Mar 2023 00:39:57 +0200
Subject: [PATCH] Dpkg::Substvars: Handle exponential expansion gracefully
We should switch our excessive recursive prevention check to cover
exponential expansion. Because with exponential expansion we always
make progress, the current check misses those cases, so we track
expansion per variable, and ignore those variables that do not contain
any dollar symbol as these cannot then be involved in further expansion.
In this context this is not considered a security issue, but a
robustness issue, where we do not want the code to end up consuming
boundless amounts of memory.
---
scripts/Dpkg/Substvars.pm | 23 +++++++++--------
scripts/t/Dpkg_Substvars.t | 52 +++++++++++++++++++++++++++++++++++---
2 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/scripts/Dpkg/Substvars.pm b/scripts/Dpkg/Substvars.pm
index d3a1fff72..7cced4d56 100644
--- a/scripts/Dpkg/Substvars.pm
+++ b/scripts/Dpkg/Substvars.pm
@@ -47,6 +47,7 @@ use constant {
SUBSTVAR_ATTR_AUTO => 2,
SUBSTVAR_ATTR_AGED => 4,
SUBSTVAR_ATTR_OPT => 8,
+ SUBSTVAR_ATTR_DEEP => 16,
};
=head1 METHODS
@@ -102,6 +103,7 @@ sub set {
my ($self, $key, $value, $attr) = @_;
$attr //= 0;
+ $attr |= SUBSTVAR_ATTR_DEEP if length $value && $value =~ m{\$};
$self->{vars}{$key} = $value;
$self->{attr}{$key} = $attr;
@@ -321,29 +323,30 @@ Substitutes variables in $string and return the result in $newstring.
sub substvars {
my ($self, $v, %opts) = @_;
+ my %seen;
my $lhs;
my $vn;
my $rhs = '';
- my $count = 0;
$opts{msg_prefix} //= $self->{msg_prefix};
$opts{no_warn} //= 0;
while ($v =~ m/^(.*?)\$\{([-:0-9a-z]+)\}(.*)$/si) {
- # If we have consumed more from the leftover data, then
- # reset the recursive counter.
- $count = 0 if (length($3) < length($rhs));
-
- if ($count >= $maxsubsts) {
- error($opts{msg_prefix} .
- g_("too many substitutions - recursive ? - in '%s'"), $v);
- }
$lhs = $1;
$vn = $2;
$rhs = $3;
+
if (defined($self->{vars}{$vn})) {
$v = $lhs . $self->{vars}{$vn} . $rhs;
$self->mark_as_used($vn);
- $count++;
+
+ if ($self->{attr}{$vn} & SUBSTVAR_ATTR_DEEP) {
+ $seen{$vn}++;
+ }
+ if (exists $seen{$vn} && $seen{$vn} >= $maxsubsts) {
+ error($opts{msg_prefix} .
+ g_("too many \${%s} substitutions (recursive?) in '%s'"),
+ $vn, $v);
+ }
if ($self->{attr}{$vn} & SUBSTVAR_ATTR_AGED) {
error($opts{msg_prefix} .
diff --git a/scripts/t/Dpkg_Substvars.t b/scripts/t/Dpkg_Substvars.t
index 92afb76de..61ac0276b 100644
--- a/scripts/t/Dpkg_Substvars.t
+++ b/scripts/t/Dpkg_Substvars.t
@@ -16,7 +16,7 @@
use strict;
use warnings;
-use Test::More tests => 55;
+use Test::More tests => 56;
use Test::Dpkg qw(:paths);
use Dpkg ();
@@ -171,10 +171,56 @@ eval {
};
$output = $@ // q{};
is($output,
- 'Dpkg_Substvars.t: error: test too many substitutions ' .
- "- recursive ? - in 'Cycle reference \${ref2}'\n",
+ 'Dpkg_Substvars.t: error: test too many ${ref0} substitutions ' .
+ "(recursive?) in 'Cycle reference \${ref1}'\n",
'recursive cyclic expansion is limited');
+# Recursive replace: Billion laughs.
+$s->set('ex0', ':)');
+$s->set('ex1', '${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}');
+$s->set('ex2', '${ex1}${ex1}${ex1}${ex1}${ex1}${ex1}${ex1}${ex1}${ex1}${ex1}');
+$s->set('ex3', '${ex2}${ex2}${ex2}${ex2}${ex2}${ex2}${ex2}${ex2}${ex2}${ex2}');
+$s->set('ex4', '${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}');
+$s->set('ex5', '${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}');
+$s->set('ex6', '${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}');
+$s->set('ex7', '${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}');
+$s->set('ex8', '${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}');
+$s->set('ex9', '${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}');
+
+eval {
+ $s->substvars('Billion laughs ${ex9}');
+ 1;
+};
+$output = $@ // q{};
+is($output,
+ 'Dpkg_Substvars.t: error: test too many ${ex1} substitutions ' .
+ "(recursive?) in 'Billion laughs :):):):):):):):):):):):):):)" .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ ':):):):):):):):):):):):):):):):):):):):):):):):):):)' .
+ '${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}${ex0}' .
+ '${ex2}${ex2}${ex2}${ex2}${ex2}' .
+ '${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}${ex3}' .
+ '${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}${ex4}' .
+ '${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}${ex5}' .
+ '${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}${ex6}' .
+ '${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}${ex7}' .
+ '${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}${ex8}' .
+ "'\n",
+ 'recursive or exponential expansion is limited');
# Strange input
is($s->substvars('Nothing to $ ${substitute here}, is it ${}?, it ${is'),
--
GitLab