Assertion failure in Perl_sv_vcatpvfn_flags (sv.c:13127)

[p5pRT]

Migrated from rt.perl.org#133913 (status was 'pending release')

Searchable as RT133913$

[p5pRT]

From @dur-randir

Created by @dur-randir

While fuzzing perl v5.29.8-21-gde59f38ed9 built with afl and run
under libdislocator, I found the following program

printf q)%7000000000E)=>

to cause an assertion failure

perl​: sv.c​:13127​: Perl_sv_vcatpvfn_flags​: Assertion `elen >= width' failed.

GDB stack trace is following​:

#0 __GI_raise (sig=sig@​entry=6) at ../sysdeps/unix/sysv/linux/raise.c​:50
#1 0x00007ffff7c25535 in __GI_abort () at abort.c​:79
#2 0x00007ffff7c2540f in __assert_fail_base (fmt=0x7ffff7d87ee0
"%s%s%s​:%u​: %s%sAssertion `%s' failed.\n%n", assertion=0x555555aa0f0e
"elen >= width",
  file=0x555555a8ff60 "sv.c", line=13189, function=<optimized out>)
at assert.c​:92
#3 0x00007ffff7c330f2 in __GI___assert_fail (assertion=0x555555aa0f0e
"elen >= width", file=0x555555a8ff60 "sv.c", line=13189,
  function=0x555555aa33b0 <__PRETTY_FUNCTION__.23320>
"Perl_sv_vcatpvfn_flags") at assert.c​:101
#4 0x00005555557c2b6b in Perl_sv_vcatpvfn_flags (sv=0x555555b2d740,
pat=0x555555b54950 "%7000000000E", patlen=12, args=0x0,
svargs=0x555555b2fdb0,
  sv_count=0, maybe_tainted=0x7fffffffddd7, flags=0) at sv.c​:13189
#5 0x00005555557bb9a8 in Perl_sv_vsetpvfn (sv=0x555555b2d740,
pat=0x555555b54950 "%7000000000E", patlen=12, args=0x0,
svargs=0x555555b2fdb0, sv_count=0,
  maybe_tainted=0x7fffffffddd7) at sv.c​:10977
#6 0x000055555586a67c in Perl_do_sprintf (sv=0x555555b2d740, len=1,
sarg=0x555555b2fda8) at doop.c​:734
#7 0x00005555558485c3 in Perl_pp_prtf () at pp_sys.c​:1628
#8 0x00005555556f6ff4 in Perl_runops_debug () at dump.c​:2537
#9 0x00005555555da157 in S_run_body (oldscope=1) at perl.c​:2692
#10 0x00005555555d96d5 in perl_run (my_perl=0x555555b2b260) at perl.c​:2615
#11 0x000055555558e14e in main (argc=2, argv=0x7fffffffe1e8,
env=0x7fffffffe200) at perlmain.c​:127

This is a regression between 5.26 and 5.28, bisect points to

commit 5860c1e
Author​: David Mitchell <davem@​iabyn.com>
Date​: Mon May 29 09​:59​:16 2017 +0100

  S_expect_number()​: return STRLEN not I32

  This static function is used by Perl_sv_vcatpvfn_flags() to read in
  a width or explicit argument number. It currently returns an I32 result
  (and croaks if the number exceeds the maximum possible I32 value).

  Change it to return STRLEN, and to croak on the value being greater than
  max(STRLEN) / 4.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.29.9:

Configured by dur-randir at Wed Feb 27 14:51:01 MSK 2019.

Summary of my perl5 (revision 5 version 29 subversion 9) configuration:
  Commit id: c1e47bad34ce1d9c84ed57c9b8978bcbd5a02e98
  Platform:
    osname=darwin
    osvers=13.4.0
    archname=darwin-thread-multi-2level
    uname='darwin isengard.local 13.4.0 darwin kernel version 13.4.0:
mon jan 11 18:17:34 pst 2016; root:xnu-2422.115.15~1release_x86_64
x86_64 '
    config_args='-de -Dusedevel -DDEBUGGING -Dusethreads'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=define
    usemultiplicity=define
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
    bincompat5005=undef
  Compiler:
    cc='cc'
    ccflags ='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9
-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include -DPERL_USE_SAFE_PUTENV'
    optimize='-O3 -g'
    cppflags='-fno-common -DPERL_DARWIN -mmacosx-version-min=10.9
-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector
-I/usr/local/include'
    ccversion=''
    gccversion='4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='cc'
    ldflags =' -mmacosx-version-min=10.9 -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/clang/6.0/lib
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib
/usr/lib
    libs=-lpthread -lgdbm -ldbm -ldl -lm -lutil -lc
    perllibs=-lpthread -ldl -lm -lutil -lc
    libc=
    so=dylib
    useshrplib=false
    libperl=libperl.a
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=bundle
    d_dlsymun=undef
    ccdlflags=' '
    cccdlflags=' '
    lddlflags=' -mmacosx-version-min=10.9 -bundle -undefined
dynamic_lookup -L/usr/local/lib -fstack-protector'



@INC for perl 5.29.9:
    lib
    /usr/local/lib/perl5/site_perl/5.29.9/darwin-thread-multi-2level
    /usr/local/lib/perl5/site_perl/5.29.9
    /usr/local/lib/perl5/5.29.9/darwin-thread-multi-2level
    /usr/local/lib/perl5/5.29.9


Environment for perl 5.29.9:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/dur-randir
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/texbin
    PERLBREW_HOME=/Users/dur-randir/.perlbrew
    PERLBREW_MANPATH=/Users/dur-randir/perlbrew/perls/perl-5.22.1/man
    PERLBREW_PATH=/Users/dur-randir/perlbrew/bin:/Users/dur-randir/perlbrew/perls/perl-5.22.1/bin
    PERLBREW_PERL=perl-5.22.1
    PERLBREW_ROOT=/Users/dur-randir/perlbrew
    PERLBREW_SHELLRC_VERSION=0.84
    PERLBREW_VERSION=0.84
    PERL_BADLANG (unset)
    SHELL=/usr/local/bin/zsh

[p5pRT]

From @tonycoz

On Fri, 08 Mar 2019 15​:36​:30 -0800, randir wrote​:

While fuzzing perl v5.29.8-21-gde59f38ed9 built with afl and run
under libdislocator, I found the following program

printf q)%7000000000E)=>

to cause an assertion failure

perl​: sv.c​:13127​: Perl_sv_vcatpvfn_flags​: Assertion `elen >= width'
failed.

GDB stack trace is following​:

#0 __GI_raise (sig=sig@​entry=6) at
../sysdeps/unix/sysv/linux/raise.c​:50
#1 0x00007ffff7c25535 in __GI_abort () at abort.c​:79
#2 0x00007ffff7c2540f in __assert_fail_base (fmt=0x7ffff7d87ee0
"%s%s%s​:%u​: %s%sAssertion `%s' failed.\n%n", assertion=0x555555aa0f0e
"elen >= width",
file=0x555555a8ff60 "sv.c", line=13189, function=<optimized out>)
at assert.c​:92
#3 0x00007ffff7c330f2 in __GI___assert_fail (assertion=0x555555aa0f0e
"elen >= width", file=0x555555a8ff60 "sv.c", line=13189,
function=0x555555aa33b0 <__PRETTY_FUNCTION__.23320>
"Perl_sv_vcatpvfn_flags") at assert.c​:101
#4 0x00005555557c2b6b in Perl_sv_vcatpvfn_flags (sv=0x555555b2d740,
pat=0x555555b54950 "%7000000000E", patlen=12, args=0x0,
svargs=0x555555b2fdb0,
sv_count=0, maybe_tainted=0x7fffffffddd7, flags=0) at sv.c​:13189
#5 0x00005555557bb9a8 in Perl_sv_vsetpvfn (sv=0x555555b2d740,
pat=0x555555b54950 "%7000000000E", patlen=12, args=0x0,
svargs=0x555555b2fdb0, sv_count=0,
maybe_tainted=0x7fffffffddd7) at sv.c​:10977
#6 0x000055555586a67c in Perl_do_sprintf (sv=0x555555b2d740, len=1,
sarg=0x555555b2fda8) at doop.c​:734
#7 0x00005555558485c3 in Perl_pp_prtf () at pp_sys.c​:1628
#8 0x00005555556f6ff4 in Perl_runops_debug () at dump.c​:2537
#9 0x00005555555da157 in S_run_body (oldscope=1) at perl.c​:2692
#10 0x00005555555d96d5 in perl_run (my_perl=0x555555b2b260) at
perl.c​:2615
#11 0x000055555558e14e in main (argc=2, argv=0x7fffffffe1e8,
env=0x7fffffffe200) at perlmain.c​:127

This is a regression between 5.26 and 5.28, bisect points to

commit 5860c1e
Author​: David Mitchell <davem@​iabyn.com>
Date​: Mon May 29 09​:59​:16 2017 +0100

S_expect_number()​: return STRLEN not I32

This static function is used by Perl_sv_vcatpvfn_flags() to read in
a width or explicit argument number. It currently returns an I32
result
(and croaks if the number exceeds the maximum possible I32 value).

Change it to return STRLEN, and to croak on the value being greater
than
max(STRLEN) / 4.

The bug causing this is this code​:

  if (width) {
  base = width;
  do { *--ptr = '0' + (base % 10); } while (base /= 10);
  }

since base is an int, the very large 7000000000 becomes a largish negative
number breaking the formatting of the width.

But fixing that reveals a different issue.

The return type of v?snprintf() is int, so such a large result can't be reported
by v?snprintf() anyway.

So fail earlier than this by checking the expected width fits in an int.

Tony

[p5pRT]

From @tonycoz

0001-perl-133913-limit-numeric-format-results-to-INT_MAX.patch

From 24d96eaa52130354efe282f202083d6e8dd7549d Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 20 Mar 2019 16:47:49 +1100
Subject: (perl #133913) limit numeric format results to INT_MAX

The return value of v?snprintf() is int, and we pay attention to that
return value, so limit the expected size of numeric formats to
INT_MAX.
---
 pod/perldiag.pod | 6 ++++++
 sv.c             | 7 +++++++
 t/op/sprintf2.t  | 7 +++++++
 3 files changed, 20 insertions(+)

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 8163dde583..125afe66b4 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -4346,6 +4346,12 @@ the meantime, try using scientific notation (e.g. "1e6" instead of
 a number.  This happens, for example with C<\o{}>, with no number between
 the braces.
 
+=item Numeric format result too large
+
+(F) The length of the result of a numeric format supplied to sprintf()
+or printf() would have been too large for the underlying C function to
+report.  This limit is typically 2GB.
+
 =item Octal number > 037777777777 non-portable
 
 (W portable) The octal number you specified is larger than 2**32-1
diff --git a/sv.c b/sv.c
index e9a46827d3..53dad06889 100644
--- a/sv.c
+++ b/sv.c
@@ -13080,6 +13080,13 @@ Perl_sv_vcatpvfn_flags(pTHX_ SV *const sv, const char *const pat, const STRLEN p
 	    if (float_need < width)
 		float_need = width;
 
+            if (float_need > INT_MAX) {
+                /* snprintf() returns an int, and we use that return value,
+                   so die horribly if the expected size is too large for int
+                */
+                Perl_croak(aTHX_ "Numeric format result too large");
+            }
+
 	    if (PL_efloatsize <= float_need) {
                 /* PL_efloatbuf should be at least 1 greater than
                  * float_need to allow a trailing \0 to be returned by
diff --git a/t/op/sprintf2.t b/t/op/sprintf2.t
index 3f4c126c68..eda4e67e99 100644
--- a/t/op/sprintf2.t
+++ b/t/op/sprintf2.t
@@ -1141,4 +1141,11 @@ foreach(
     is sprintf("%.0f", $_), sprintf("%-.0f", $_), "special-case %.0f on $_";
 }
 
+# large uvsize needed so the large width is parsed properly
+# large sizesize needed so the STRLEN check doesn't
+if ($Config{intsize} == 4 && $Config{uvsize} > 4 && $Config{sizesize} > 4) {
+    eval { my $x = sprintf("%7000000000E", 0) };
+    like($@, qr/^Numeric format result too large at /,
+         "croak for very large numeric format results");
+}
 done_testing();
-- 
2.11.0

[p5pRT]

The RT System itself - Status changed from 'new' to 'open'

[p5pRT]

From @iabyn

On Tue, Mar 19, 2019 at 11​:01​:08PM -0700, Tony Cook via RT wrote​:

On Fri, 08 Mar 2019 15​:36​:30 -0800, randir wrote​:

While fuzzing perl v5.29.8-21-gde59f38ed9 built with afl and run
under libdislocator, I found the following program

printf q)%7000000000E)=>

to cause an assertion failure

perl​: sv.c​:13127​: Perl_sv_vcatpvfn_flags​: Assertion `elen >= width'
failed.
The bug causing this is this code​:

    if \(width\) \{
        base = width;
        do \{ \*\-\-ptr = '0' \+ \(base % 10\); \} while \(base /= 10\);
    \}

since base is an int, the very large 7000000000 becomes a largish negative
number breaking the formatting of the width.

But fixing that reveals a different issue.

The return type of v?snprintf() is int, so such a large result can't be reported
by v?snprintf() anyway.

So fail earlier than this by checking the expected width fits in an int.

Your suggested patch looks good to me.

--
Any [programming] language that doesn't occasionally surprise the
novice will pay for it by continually surprising the expert.
  -- Larry Wall

[p5pRT]

From @dur-randir

Can this patch be applied now?

[p5pRT]

From @tonycoz

On Wed, 01 May 2019 05​:55​:57 -0700, davem wrote​:

On Tue, Mar 19, 2019 at 11​:01​:08PM -0700, Tony Cook via RT wrote​:

On Fri, 08 Mar 2019 15​:36​:30 -0800, randir wrote​:

While fuzzing perl v5.29.8-21-gde59f38ed9 built with afl and run
under libdislocator, I found the following program

printf q)%7000000000E)=>

to cause an assertion failure

perl​: sv.c​:13127​: Perl_sv_vcatpvfn_flags​: Assertion `elen >= width'
failed.
The bug causing this is this code​:

if (width) {
base = width;
do { *--ptr = '0' + (base % 10); } while (base /= 10);
}

since base is an int, the very large 7000000000 becomes a largish
negative
number breaking the formatting of the width.

But fixing that reveals a different issue.

The return type of v?snprintf() is int, so such a large result can't
be reported
by v?snprintf() anyway.

So fail earlier than this by checking the expected width fits in an
int.

Your suggested patch looks good to me.

Applied in 027471c.

Thanks for the report and the reminder.

Tony

[p5pRT]

@tonycoz - Status changed from 'open' to 'pending release'