Skip to content

Add limits to the size of the string repetition multiplier #23561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: blead
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions op.c
Original file line number Diff line number Diff line change
Expand Up @@ -5011,6 +5011,38 @@ S_fold_constants(pTHX_ OP *const o)
break;
case OP_REPEAT:
if (o->op_private & OPpREPEAT_DOLIST) goto nope;
/* Croak if the string is going to be unrealistically
* large. (GH#13324) Otherwise, don't constant fold
* above a certain threshold. (GH#13793 & GH#20586)
*
* Implementation note: pp_pow returns powers of 2 as an NV
* e.g. my $x = "A" x (2**3);
*/
if (OP_TYPE_IS(cBINOPo->op_last, OP_CONST)) {
SV *constsv = cSVOPx_sv(cBINOPo->op_last);
UV arbitrary = 1024 * 1024;

if (SvIOKp(constsv)) {
if (SvIOK_UV(constsv)) {
if (SvUVX(constsv) > SIZE_MAX >> 2)
ck_warner(packWARN(WARN_MISC), "Unrealistically large string repetition value");
if (SvUVX(constsv) > arbitrary)
goto nope;
} else {
if (SvIVX(constsv) > (IV)(SIZE_MAX >> 2))
ck_warner(packWARN(WARN_MISC), "Unrealistically large string repetition value");
if (SvIVX(constsv) > (IV)arbitrary)
goto nope;
}
} else {
NV rhs = 0.0; rhs = SvNV_nomg(constsv);
if (rhs >= (NV)((SIZE_MAX >> 2) +1) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a -1.0 for safety? It can really questionable what the "53rd digit" to the right side of the . is, and what CC, what CPU, what OS, which security or spectre patch for your OS and CC, and make month and year of all 4 please.

I don't trust C's float/double keyword's rounding modes at all, and were constant folding intermediate values done in FP CPU real instructions or C abstract machine instructions, calculations done at 32, 64, or 80 bit or 128 bit intermediate floating point precision?

the goose has been cooked at malloc(2 GB) or malloc(2GB-1 byte) either way. thats not a future bug ticket.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also don't forget that Intel 64/AMD 64 in 64 bit mode CPUs are incapable of doing 80 bit floating pointer intermediate math unlike 32 bit mode. So >= 2^53 or >= 2^52 starts introducing more and more "error" or rounding into the math formula, and we have a 64 bit memory space on paper (more like 48 bits unless your a rack server of brand new Xeons, which I think finally took another chomp at the AMD64 ISA's central address space gap).

ck_warner(packWARN(WARN_MISC), "Unrealistically large string repetition value");
}
if (rhs > (NV)arbitrary)
goto nope;
}
}
break;
case OP_SREFGEN:
if (cUNOPx(cUNOPo->op_first)->op_first->op_type != OP_CONST
Expand Down
12 changes: 12 additions & 0 deletions pod/perldiag.pod
Original file line number Diff line number Diff line change
Expand Up @@ -7385,6 +7385,18 @@ reserved word. It's best to put such a word in quotes, or capitalize it
somehow, or insert an underbar into it. You might also declare it as a
subroutine.

=item Unrealistically large string repetition value

(W misc) The value of the right operand in the string repetition operator is
likely close to or will exceed the maximum memory allocation that
your system can provide.

Even if an allocation of this size does succeed, subsequent string
copies may still result in an out-of-memory condition.

Note that a smaller memory constraint might be imposed on your
application under C<ulimit>, if containerized, or other local configuration.

=item Unrecognized character %s; marked by S<<-- HERE> after %s near column
%d

Expand Down
10 changes: 9 additions & 1 deletion t/op/repeat.t
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ BEGIN {
set_up_inc( '../lib' );
}

plan(tests => 50);
plan(tests => 51);

# compile time

Expand Down Expand Up @@ -193,6 +193,14 @@ fresh_perl_like(
eval q{() = (() or ((0) x 0)); 1};
is($@, "", "RT #130247");

# [GH #13324] Perl croaks if a string repetition seems unsupportable
fresh_perl_like(
'use warnings; my $x = "A" x (2**99)',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we try some maximum toxic 0.0 NV literals here? maybe creating them with PP pack and or unpack.

qr/Unrealistically large string repetition/,
{stderr => 1},
'Warn on unrealistically large string repetition',
);

# yes, the newlines matter
fresh_perl_is(<<'PERL', "", { stderr => 1 }, "(perl #133778) MARK mishandling");
map{s[][];eval;0}<DATA>__END__
Expand Down
7 changes: 7 additions & 0 deletions t/perf/opcount.t
Original file line number Diff line number Diff line change
Expand Up @@ -1245,4 +1245,11 @@ test_opcount(0, "Empty else{} blocks are optimised away",
stub => 0
});

# GH #13793, GH #20586
test_opcount(0, "Don't fold string repetition once deeemed too large",
sub { my $x = "A" x (2**22) },
{
repeat => 1,
});

done_testing();
Loading