Skip to content

Commit

Permalink
sv_gets: use a SSize_t append character index
Browse files Browse the repository at this point in the history
This used an I32, which could overflow when reading from a large
source.

https://www.perlmonks.org/?node_id=11161665
  • Loading branch information
tonycoz committed Sep 12, 2024
1 parent bd12780 commit 5553555
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 13 deletions.
1 change: 1 addition & 0 deletions MANIFEST
Original file line number Diff line number Diff line change
Expand Up @@ -5972,6 +5972,7 @@ t/bigmem/stack_over.t Check handling of stack overflows with 32-bit MARK on 64
t/bigmem/str.t Test string primitives with large strings
t/bigmem/subst.t Test s/// with large strings
t/bigmem/subst2.t Test s//EXPR/e with large strings
t/bigmem/sv_gets.t Test reading a lot with sv_gets()
t/bigmem/vec.t Check vec() handles large offsets
t/charset_tools.pl To aid in portable testing across platforms with different character sets
t/class/accessor.t See if accessor methods work
Expand Down
2 changes: 1 addition & 1 deletion embed.fnc
Original file line number Diff line number Diff line change
Expand Up @@ -3175,7 +3175,7 @@ ATdpx |SV * |sv_get_backrefs|NN SV * const sv
Adip |void |SvGETMAGIC |NN SV *sv
Adp |char * |sv_gets |NN SV * const sv \
|NN PerlIO * const fp \
|I32 append
|SSize_t append
Cdp |char * |sv_grow |NN SV * const sv \
|STRLEN newlen
Cdp |char * |sv_grow_fresh |NN SV * const sv \
Expand Down
2 changes: 1 addition & 1 deletion proto.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 12 additions & 11 deletions sv.c
Original file line number Diff line number Diff line change
Expand Up @@ -8680,7 +8680,7 @@ Perl_sv_collxfrm_flags(pTHX_ SV *const sv, STRLEN *const nxp, const I32 flags)
#endif /* USE_LOCALE_COLLATE */

static char *
S_sv_gets_append_to_utf8(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
S_sv_gets_append_to_utf8(pTHX_ SV *const sv, PerlIO *const fp, SSize_t append)
{
SV * const tsv = newSV_type(SVt_NULL);
ENTER;
Expand All @@ -8694,7 +8694,7 @@ S_sv_gets_append_to_utf8(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
}

static char *
S_sv_gets_read_record(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
S_sv_gets_read_record(pTHX_ SV *const sv, PerlIO *const fp, SSize_t append)
{
SSize_t bytesread;
const STRLEN recsize = SvUV(SvRV(PL_rs)); /* RsRECORD() guarantees > 0. */
Expand Down Expand Up @@ -8822,7 +8822,7 @@ in the SV (typically, C<SvCUR(sv)> is a suitable choice).
*/

char *
Perl_sv_gets(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
Perl_sv_gets(pTHX_ SV *const sv, PerlIO *const fp, SSize_t append)
{
const char *rsptr;
STRLEN rslen;
Expand All @@ -8848,7 +8848,8 @@ Perl_sv_gets(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
if (PerlIO_isutf8(fp)) {
if (!SvUTF8(sv)) {
sv_utf8_upgrade_nomg(sv);
sv_pos_u2b(sv,&append,0);
append = (SSize_t)sv_pos_u2b_flags(sv, (STRLEN)append, NULL,
SV_GMAGIC|SV_CONST_RETURN);
}
} else if (SvUTF8(sv)) {
return S_sv_gets_append_to_utf8(aTHX_ sv, fp, append);
Expand Down Expand Up @@ -8962,11 +8963,11 @@ Perl_sv_gets(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
STDCHAR *ptr; /* pointer into fp's read-ahead buffer */
STRLEN bpx; /* length of the data in the target sv
used to fix pointers after a SvGROW */
I32 shortbuffered; /* If the pv buffer is shorter than the amount
of data left in the read-ahead buffer.
If 0 then the pv buffer can hold the full
amount left, otherwise this is the amount it
can hold. */
SSize_t shortbuffered;/* If the pv buffer is shorter than the amount
of data left in the read-ahead buffer.
If 0 then the pv buffer can hold the full
amount left, otherwise this is the amount it
can hold. */

/* Here is some breathtakingly efficient cheating */

Expand Down Expand Up @@ -9029,11 +9030,11 @@ Perl_sv_gets(pTHX_ SV *const sv, PerlIO *const fp, I32 append)
cnt = PerlIO_get_cnt(fp);

/* make sure we have the room */
if ((I32)(SvLEN(sv) - append) <= cnt + 1) {
if ((SSize_t)(SvLEN(sv) - append) <= cnt + 1) {
/* Not room for all of it
if we are looking for a separator and room for some
*/
if (rslen && cnt > 80 && (I32)SvLEN(sv) > append) {
if (rslen && cnt > 80 && SvLEN(sv) > (STRLEN)append) {
/* just process what we have room for */
shortbuffered = cnt - SvLEN(sv) + append + 1;
cnt -= shortbuffered;
Expand Down
48 changes: 48 additions & 0 deletions t/bigmem/sv_gets.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#!perl
BEGIN {
chdir 't' if -d 't';
@INC = "../lib";
}

use strict;
require './test.pl';
use Config qw(%Config);

# 2 for the child, 2 for the parent
$ENV{PERL_TEST_MEMORY} >= 4
or skip_all("Need ~4Gb for this test");
$Config{ptrsize} >= 8
or skip_all("Need 64-bit pointers for this test");

{
# https://www.perlmonks.org/?node_id=11161665
my $x = `$^X -e "print q/x/ x 0x80000000"`;
is(length $x, 0x80000000, "check entire input read");
undef $x;
}

{
# sv_gets_append_to_utf8 append parameter
my $x = "x";
$x x= 0x8000_0000;
utf8::upgrade($x);
# using bareword handle because the rcatline optimization isn't done
# for non-barewords
# for $fh
# we chdired to "t"
open FH, "<", "TEST" or die $!;
$x .= <FH>;
pass("didn't crash appending readline to large upgraded scalar");
}

{
# sv_gets_read_record append parameter
my $x = "x";
$x x= 0x8000_0000;
open FH, "<", "TEST" or die $!;
local $/ = \100;
$x .= <FH>;
pass("didn't crash appending readline record to large scalar");
}

done_testing();

0 comments on commit 5553555

Please sign in to comment.