Skip to content

PERL-5.26.1 heap-buffer-overflow READ of size 11 #16343

Description

@p5pRT

Migrated from rt.perl.org#132655 (status was 'resolved')

Searchable as RT132655$

Activity

  1. p5pRT commented on Dec 26, 2017

    @p5pRT
    Author

    From sraums2498@gmail.com

    =================================================================
    ==2940==ERROR​: AddressSanitizer​: heap-buffer-overflow on address
    0x60200000d91a at pc 0x7eff7544d20b bp 0x7ffcc6b8d800 sp 0x7ffcc6b8cfa8
    READ of size 11 at 0x60200000d91a thread T0
      #0 0x7eff7544d20a in __interceptor_strlen
    (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x7020a)
      #1 0x2df9cfb in Perl_my_atof2
    /home/asan_perl/Documents/perl-5.26.1/numeric.c​:1349
      #2 0x2e04973 in Perl_my_atof
    /home/asan_perl/Documents/perl-5.26.1/numeric.c​:1244
      #3 0x1e9febc in S_sv_setnv
    /home/asan_perl/Documents/perl-5.26.1/sv.c​:2109
      #4 0x1e9febc in S_sv_2iuv_common
    /home/asan_perl/Documents/perl-5.26.1/sv.c​:2311
      #5 0x1ea95e7 in Perl_sv_2iv_flags
    /home/asan_perl/Documents/perl-5.26.1/sv.c​:2504
      #6 0x205229d in Perl_pp_multiply
    /home/asan_perl/Documents/perl-5.26.1/pp.c​:1373
      #7 0x1b1bc2e in Perl_runops_standard
    /home/asan_perl/Documents/perl-5.26.1/run.c​:41
      #8 0x9218a5 in S_run_body
    /home/asan_perl/Documents/perl-5.26.1/perl.c​:2519
      #9 0x9218a5 in perl_run
    /home/asan_perl/Documents/perl-5.26.1/perl.c​:2447
      #10 0x46b6a7 in main
    /home/asan_perl/Documents/perl-5.26.1/perlmain.c​:123
      #11 0x7eff746d182f in __libc_start_main
    (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
      #12 0x46c888 in _start
    (/home/asan_perl/Documents/perl-5.26.1/perl+0x46c888)

    0x60200000d91a is located 0 bytes to the right of 10-byte region
    [0x60200000d910,0x60200000d91a)
    allocated by thread T0 here​:
      #0 0x7eff75475602 in malloc
    (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
      #1 0x167dd81 in Perl_safesysmalloc
    /home/asan_perl/Documents/perl-5.26.1/util.c​:153
      #2 0x1de5477 in Perl_sv_grow
    /home/asan_perl/Documents/perl-5.26.1/sv.c​:1601
      #3 0x1e07a57 in Perl_newSV
    /home/asan_perl/Documents/perl-5.26.1/sv.c​:5682
      #4 0x2ebe18d in S_unpack_rec
    /home/asan_perl/Documents/perl-5.26.1/pp_pack.c​:1716
      #5 0x2f6a66a in Perl_unpackstring
    /home/asan_perl/Documents/perl-5.26.1/pp_pack.c​:838
      #6 0x2f6c002 in Perl_pp_unpack
    /home/asan_perl/Documents/perl-5.26.1/pp_pack.c​:1848
      #7 0x1b1bc2e in Perl_runops_standard
    /home/asan_perl/Documents/perl-5.26.1/run.c​:41
      #8 0x9218a5 in S_run_body
    /home/asan_perl/Documents/perl-5.26.1/perl.c​:2519
      #9 0x9218a5 in perl_run
    /home/asan_perl/Documents/perl-5.26.1/perl.c​:2447
      #10 0x46b6a7 in main
    /home/asan_perl/Documents/perl-5.26.1/perlmain.c​:123
      #11 0x7eff746d182f in __libc_start_main
    (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

    SUMMARY​: AddressSanitizer​: heap-buffer-overflow ??​:0 __interceptor_strlen
    Shadow bytes around the buggy address​:
      0x0c047fff9ad0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff9ae0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff9af0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff9b00​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff9b10​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    =>0x0c047fff9b20​: fa fa 00[02]fa fa 00 02 fa fa 00 02 fa fa fd fa
      0x0c047fff9b30​: fa fa 00 02 fa fa fd fd fa fa fd fd fa fa fd fd
      0x0c047fff9b40​: fa fa 06 fa fa fa fd fa fa fa 03 fa fa fa fd fd
      0x0c047fff9b50​: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fa
      0x0c047fff9b60​: fa fa 00 02 fa fa fd fd fa fa fd fd fa fa 02 fa
      0x0c047fff9b70​: fa fa 07 fa fa fa 00 03 fa fa 00 02 fa fa 00 06
    Shadow byte legend (one shadow byte represents 8 application bytes)​:
      Addressable​: 00
      Partially addressable​: 01 02 03 04 05 06 07
      Heap left redzone​: fa
      Heap right redzone​: fb
      Freed heap region​: fd
      Stack left redzone​: f1
      Stack mid redzone​: f2
      Stack right redzone​: f3
      Stack partial redzone​: f4
      Stack after return​: f5
      Stack use after scope​: f8
      Global redzone​: f9
      Global init order​: f6
      Poisoned by user​: f7
      Container overflow​: fc
      Array cookie​: ac
      Intra object redzone​: bb
      ASan internal​: fe
    ==2940==ABORTING

    --
    Regards,
    SRAUMS

  2. p5pRT commented on Dec 26, 2017

    @p5pRT
    Author
  3. p5pRT commented on Jan 9, 2018

    @p5pRT
    Author

    From @hvds

    This looks very similar to rt132654​:
      ./miniperl -e 's/(?=)/ab/; 0 + unpack "u"'

    I'll add a cross-reference, but it's a bit too early to merge them.

    Hugo

  4. p5pRT commented on Jan 9, 2018

    @p5pRT
    Author

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

  5. p5pRT commented on Jan 23, 2018

    @p5pRT
    Author

    From @tonycoz

    On Tue, 09 Jan 2018 00​:46​:06 -0800, hv wrote​:

    This looks very similar to rt132654​:
    ./miniperl -e 's/(?=)/ab/; 0 + unpack "u"'

    This can be further simplified to​:

      ./miniperl -e '0 * unpack "u", "ab"'

    I'll add a cross-reference, but it's a bit too early to merge them.

    It's unrelated - 132654 is about (mis-)use of the p pack format.

    The problem here is the "u" decoder, which creates a new SV (upgraded to SVt_PV in the case that matters) and sets POK on it.

    When the "ab" fails to decode, no changes are made to the PV, leaving it unterminated.

    pp_add (or pp_multiply, depending on whether you use Hugo's or the original's test case) then tries to use that SV as a number, eventually leading to sv_2iv_flags trying to decode the (unterminated) PV as a number via Perl_my_atof() which treats the PV as a C NUL terminated string and accesses the uninitialized bytes.

    Trivially fixed by ensuring the PV is NUL terminated.

    Inline Patch
    diff --git a/pp_pack.c b/pp_pack.c
    index 8937d6d715..50e9b30221 100644
    --- a/pp_pack.c
    +++ b/pp_pack.c
    @@ -1710,7 +1710,10 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
                if (!checksum) {
                     const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
                    sv = sv_2mortal(newSV(l));
    -               if (l) SvPOK_on(sv);
    +               if (l) {
    +                    SvPOK_on(sv);
    +                    *SvEND(sv) = '\0';
    +                }
                }
     
                 /* Note that all legal uuencoded strings are ASCII printables, so
    
    
    (alternatively we could use SvPVCLEAR(), but that does work that isn't required here.)

    I think it's highly likely there's code accepting untrusted uuencoded data and passing it through C<unpack "u">.

    There's a (low) chance of an attacker who can control the uudecoded data causing perl to SEGV if the strlen happens to run off the end of the mapped memory block.

    This should only cause perl to crash, there's nothing that will write to the memory block.

    This issue can only occur if the first byte in the string is not a uuencoding character - so no decoded data is emitted, so the attacker has zero control over what ends up the SV that's causing the problem.

    In the past we've treated similar issues as *not* being security issues.

    I don't think this is a security issue.

    Tony

  6. p5pRT commented on Apr 9, 2018

    @p5pRT
    Author

    From @iabyn

    On Mon, Jan 22, 2018 at 09​:14​:09PM -0800, Tony Cook via RT wrote​:

    On Tue, 09 Jan 2018 00​:46​:06 -0800, hv wrote​:

    This looks very similar to rt132654​:
    ./miniperl -e 's/(?=)/ab/; 0 + unpack "u"'

    This can be further simplified to​:

    ./miniperl -e '0 * unpack "u", "ab"'

    I'll add a cross-reference, but it's a bit too early to merge them.

    It's unrelated - 132654 is about (mis-)use of the p pack format.

    The problem here is the "u" decoder, which creates a new SV (upgraded to SVt_PV in the case that matters) and sets POK on it.

    When the "ab" fails to decode, no changes are made to the PV, leaving it unterminated.

    pp_add (or pp_multiply, depending on whether you use Hugo's or the original's test case) then tries to use that SV as a number, eventually leading to sv_2iv_flags trying to decode the (unterminated) PV as a number via Perl_my_atof() which treats the PV as a C NUL terminated string and accesses the uninitialized bytes.

    Trivially fixed by ensuring the PV is NUL terminated.

    diff --git a/pp_pack.c b/pp_pack.c
    index 8937d6d715..50e9b30221 100644
    --- a/pp_pack.c
    +++ b/pp_pack.c
    @​@​ -1710,7 +1710,10 @​@​ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
    if (!checksum) {
    const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
    sv = sv_2mortal(newSV(l));
    - if (l) SvPOK_on(sv);
    + if (l) {
    + SvPOK_on(sv);
    + *SvEND(sv) = '\0';
    + }
    }

             /\* Note that all legal uuencoded strings are ASCII printables\, so
    

    (alternatively we could use SvPVCLEAR(), but that does work that isn't required here.)

    I think it's highly likely there's code accepting untrusted uuencoded data and passing it through C<unpack "u">.

    There's a (low) chance of an attacker who can control the uudecoded data causing perl to SEGV if the strlen happens to run off the end of the mapped memory block.

    This should only cause perl to crash, there's nothing that will write to the memory block.

    This issue can only occur if the first byte in the string is not a uuencoding character - so no decoded data is emitted, so the attacker has zero control over what ends up the SV that's causing the problem.

    In the past we've treated similar issues as *not* being security issues.

    I don't think this is a security issue.

    Agreed. I think you should go ahead apply the patch.

    --
    The Enterprise is involved in a bizarre time-warp experience which is in
    some way unconnected with the Late 20th Century.
      -- Things That Never Happen in "Star Trek" #14

  7. p5pRT commented on Jun 25, 2018

    @p5pRT
    Author

    From @geeknik

    This overflow was triggered while fuzzing Perl v5.28.0-RC2-2-g197e798,
    compiled with ASan and Clang 7.0.0 (trunk 335091).

    echo "dW5wYWNrInV1LwAiLCQw" | base64 -d | tee test099.pl ; ./perl test099.pl

    ==22268==ERROR​: AddressSanitizer​: heap-buffer-overflow on address
    0x602000000f5a at pc 0x00000044a6a9 bp 0x7ffeec648830 sp 0x7ffeec647fd8
    READ of size 11 at 0x602000000f5a thread T0
      #0 0x44a6a8 in __interceptor_strlen
    /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc​:356​:5
      #1 0xac75e4 in Perl_my_atof2 /root/perl/numeric.c​:1373​:28
      #2 0xac6947 in Perl_my_atof /root/perl/numeric.c​:1269​:13
      #3 0x8c694e in S_sv_setnv /root/perl/sv.c​:2115​:9
      #4 0x856158 in S_sv_2iuv_common /root/perl/sv.c​:2317​:13
      #5 0x8539c1 in Perl_sv_2iv_flags /root/perl/sv.c​:2510​:6
      #6 0xae8a76 in S_unpack_rec /root/perl/pp_pack.c​:1828​:23
      #7 0xadfa5a in Perl_unpackstring /root/perl/pp_pack.c​:851​:12
      #8 0xaef641 in Perl_pp_unpack /root/perl/pp_pack.c​:1861​:11
      #9 0x77aaad in Perl_runops_debug /root/perl/dump.c​:2536​:23
      #10 0x5a16d7 in S_run_body /root/perl/perl.c
      #11 0x5a0dcd in perl_run /root/perl/perl.c​:2617​:2
      #12 0x50dea0 in main /root/perl/perlmain.c​:122​:9
      #13 0x7f3bc2fe582f in __libc_start_main
    /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c​:291
      #14 0x4371a8 in _start (/root/perl/perl+0x4371a8)

    0x602000000f5a is located 0 bytes to the right of 10-byte region
    [0x602000000f50,0x602000000f5a)
    allocated by thread T0 here​:
      #0 0x4df0a3 in malloc
    /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc​:146​:3
      #1 0x77f0ed in Perl_safesysmalloc /root/perl/util.c​:153​:21
      #2 0x84b9b7 in Perl_sv_grow /root/perl/sv.c​:1603​:17
      #3 0x882555 in Perl_newSV /root/perl/sv.c​:5601​:2
      #4 0xae6d85 in S_unpack_rec /root/perl/pp_pack.c​:1729​:8
      #5 0xadfa5a in Perl_unpackstring /root/perl/pp_pack.c​:851​:12
      #6 0xaef641 in Perl_pp_unpack /root/perl/pp_pack.c​:1861​:11
      #7 0x77aaad in Perl_runops_debug /root/perl/dump.c​:2536​:23
      #8 0x5a16d7 in S_run_body /root/perl/perl.c
      #9 0x5a0dcd in perl_run /root/perl/perl.c​:2617​:2
      #10 0x50dea0 in main /root/perl/perlmain.c​:122​:9
      #11 0x7f3bc2fe582f in __libc_start_main
    /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c​:291

    SUMMARY​: AddressSanitizer​: heap-buffer-overflow
    /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc​:356​:5
    in __interceptor_strlen

  8. p5pRT commented on Jun 25, 2018

    @p5pRT
    Author

    From @iabyn

    On Mon, Jun 25, 2018 at 12​:11​:33PM -0700, Brian Carpenter wrote​:

    # New Ticket Created by Brian Carpenter
    # Please include the string​: [perl #133305]
    # in the subject line of all future correspondence about this issue.
    # <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133305 >

    This overflow was triggered while fuzzing Perl v5.28.0-RC2-2-g197e798,
    compiled with ASan and Clang 7.0.0 (trunk 335091).

    echo "dW5wYWNrInV1LwAiLCQw" | base64 -d | tee test099.pl ; ./perl test099.pl

    Or more prozaically,

    $ valgrind ./perl -e'unpack "uu/\0", "abc";'

    The direct fault is that the 'u' action is creating an SV with a
    zero-length string *without* a terminating \0, and the '/' action
    (which I don't fully understand and the description in perlfunc makes my
    eyes bleed) causes that string to be popped off the stack and treated as a
    number, which eventually causes the string to passed to atof(), which
    because it isn't null terminated, reads garbage off the end.

    I'm not yet sure how serious it is.

    --
    "Foul and greedy Dwarf - you have eaten the last candle."
      -- "Hordes of the Things", BBC Radio.

  9. p5pRT commented on Jun 25, 2018

    @p5pRT
    Author

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

  10. p5pRT commented on Jun 26, 2018

    @p5pRT
    Author

    From @khwilliamson

    On 06/25/2018 05​:07 PM, Dave Mitchell wrote​:

    On Mon, Jun 25, 2018 at 12​:11​:33PM -0700, Brian Carpenter wrote​:

    # New Ticket Created by Brian Carpenter
    # Please include the string​: [perl #133305]
    # in the subject line of all future correspondence about this issue.
    # <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133305 >

    This overflow was triggered while fuzzing Perl v5.28.0-RC2-2-g197e798,
    compiled with ASan and Clang 7.0.0 (trunk 335091).

    echo "dW5wYWNrInV1LwAiLCQw" | base64 -d | tee test099.pl ; ./perl test099.pl

    Or more prozaically,

    $ valgrind ./perl -e'unpack "uu/\0", "abc";'

    The direct fault is that the 'u' action is creating an SV with a
    zero-length string *without* a terminating \0, and the '/' action
    (which I don't fully understand and the description in perlfunc makes my
    eyes bleed) causes that string to be popped off the stack and treated as a
    number, which eventually causes the string to passed to atof(), which
    because it isn't null terminated, reads garbage off the end.

    I'm not yet sure how serious it is.

    See also
    https://rt.perl.org/Public/Bug/Display.html?id=131642

  11. p5pRT commented on Jun 27, 2018

    @p5pRT
    Author

    From @iabyn

    On Tue, Jun 26, 2018 at 12​:07​:36AM +0100, Dave Mitchell wrote​:

    On Mon, Jun 25, 2018 at 12​:11​:33PM -0700, Brian Carpenter wrote​:

    # New Ticket Created by Brian Carpenter
    # Please include the string​: [perl #133305]
    # in the subject line of all future correspondence about this issue.
    # <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=133305 >

    This overflow was triggered while fuzzing Perl v5.28.0-RC2-2-g197e798,
    compiled with ASan and Clang 7.0.0 (trunk 335091).

    echo "dW5wYWNrInV1LwAiLCQw" | base64 -d | tee test099.pl ; ./perl test099.pl

    Or more prozaically,

    $ valgrind ./perl -e'unpack "uu/\0", "abc";'

    The direct fault is that the 'u' action is creating an SV with a
    zero-length string *without* a terminating \0, and the '/' action
    (which I don't fully understand and the description in perlfunc makes my
    eyes bleed) causes that string to be popped off the stack and treated as a
    number, which eventually causes the string to passed to atof(), which
    because it isn't null terminated, reads garbage off the end.

    I'm not yet sure how serious it is.

    In general, unpack('u', 'nonlegal-uuencode-chars') will return
    an SV which is POK, length 0, but which *doesn't* have a terminating \0
    at position 0 in the string buffer. If that SV is passed to something that
    uses strlen() or equivalent rather than SvCUR(), it may overrun the
    buffer; for example​:

      0.0 + unpack("u", "abc");

    trips up in atof().

    It's very plausible that an attacker can provide such malformed uuencoded
    text; it's unlikely that an attacker could find perl code which uses the
    return value of unpack in a way that relies on the \0 in a dangerous way.

    Typically you would expect the result of unpack('u') to be fed to
    a concat, a print, or a match etc. All these are safe against
    non-\0-terminated strings.

    My example above of treating the uudecoded result as a number is, I think,
    unlikely. The original failing example for this ticket, unpack "uu/\0", ..
    uses a nonsensical unpack template which wouldn't appear in nature.

    So I think I should just fix it (the fix is trivial) and not treat it as a
    security issue.

    --
    This is a great day for France!
      -- Nixon at Charles De Gaulle's funeral

  12. p5pRT commented on Aug 20, 2018

    @p5pRT
    Author

    From @tonycoz

    On Wed, 27 Jun 2018 01​:37​:07 -0700, davem wrote​:

    In general, unpack('u', 'nonlegal-uuencode-chars') will return
    an SV which is POK, length 0, but which *doesn't* have a terminating
    \0
    at position 0 in the string buffer. If that SV is passed to something
    that
    uses strlen() or equivalent rather than SvCUR(), it may overrun the
    buffer; for example​:

    0.0 + unpack("u", "abc");

    trips up in atof().

    I believe this is the same issue as #132655.

    Tony

  13. p5pRT commented on Aug 20, 2018

    @p5pRT
    Author

    From @tonycoz

    On Mon, 09 Apr 2018 08​:53​:03 -0700, davem wrote​:

    On Mon, Jan 22, 2018 at 09​:14​:09PM -0800, Tony Cook via RT wrote​:

    On Tue, 09 Jan 2018 00​:46​:06 -0800, hv wrote​:

    This looks very similar to rt132654​:
    ./miniperl -e 's/(?=)/ab/; 0 + unpack "u"'

    This can be further simplified to​:

    ./miniperl -e '0 * unpack "u", "ab"'

    I'll add a cross-reference, but it's a bit too early to merge them.

    It's unrelated - 132654 is about (mis-)use of the p pack format.

    The problem here is the "u" decoder, which creates a new SV (upgraded
    to SVt_PV in the case that matters) and sets POK on it.

    When the "ab" fails to decode, no changes are made to the PV, leaving
    it unterminated.

    pp_add (or pp_multiply, depending on whether you use Hugo's or the
    original's test case) then tries to use that SV as a number,
    eventually leading to sv_2iv_flags trying to decode the
    (unterminated) PV as a number via Perl_my_atof() which treats the PV
    as a C NUL terminated string and accesses the uninitialized bytes.

    Trivially fixed by ensuring the PV is NUL terminated.

    diff --git a/pp_pack.c b/pp_pack.c
    index 8937d6d715..50e9b30221 100644
    --- a/pp_pack.c
    +++ b/pp_pack.c
    @​@​ -1710,7 +1710,10 @​@​ S_unpack_rec(pTHX_ tempsym_t* symptr, const
    char *s, const char *strbeg, const c
    if (!checksum) {
    const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
    sv = sv_2mortal(newSV(l));
    - if (l) SvPOK_on(sv);
    + if (l) {
    + SvPOK_on(sv);
    + *SvEND(sv) = '\0';
    + }
    }

    /* Note that all legal uuencoded strings are ASCII printables, so

    (alternatively we could use SvPVCLEAR(), but that does work that
    isn't required here.)

    I think it's highly likely there's code accepting untrusted uuencoded
    data and passing it through C<unpack "u">.

    There's a (low) chance of an attacker who can control the uudecoded
    data causing perl to SEGV if the strlen happens to run off the end of
    the mapped memory block.

    This should only cause perl to crash, there's nothing that will write
    to the memory block.

    This issue can only occur if the first byte in the string is not a
    uuencoding character - so no decoded data is emitted, so the attacker
    has zero control over what ends up the SV that's causing the problem.

    In the past we've treated similar issues as *not* being security
    issues.

    I don't think this is a security issue.

    Agreed. I think you should go ahead apply the patch.

    Here's the patch, I'll apply it in a couple of days, unless someone suggests we make this a security issue.

    Tony

  14. p5pRT commented on Aug 20, 2018

    @p5pRT
    Author

    From @tonycoz

    0001-perl-132655-nul-terminate-result-of-unpack-u-of-inva.patch
    From 49bb9eff715af452bcbeb2f94716891d185a0f54 Mon Sep 17 00:00:00 2001
    From: Tony Cook <tony@develop-help.com>
    Date: Mon, 20 Aug 2018 16:31:45 +1000
    Subject: (perl #132655) nul terminate result of unpack "u" of invalid data
    
    In the given test case, Perl_atof2() would run off the end of the PV,
    producing an error from ASAN.
    ---
     pp_pack.c   | 5 ++++-
     t/op/pack.t | 9 ++++++++-
     2 files changed, 12 insertions(+), 2 deletions(-)
    
    diff --git a/pp_pack.c b/pp_pack.c
    index 5e9cc64301..f8be9d48ae 100644
    --- a/pp_pack.c
    +++ b/pp_pack.c
    @@ -1727,7 +1727,10 @@ S_unpack_rec(pTHX_ tempsym_t* symptr, const char *s, const char *strbeg, const c
     	    if (!checksum) {
                     const STRLEN l = (STRLEN) (strend - s) * 3 / 4;
     		sv = sv_2mortal(newSV(l));
    -		if (l) SvPOK_on(sv);
    +		if (l) {
    +                    SvPOK_on(sv);
    +                    *SvEND(sv) = '\0';
    +                }
     	    }
     
                 /* Note that all legal uuencoded strings are ASCII printables, so
    diff --git a/t/op/pack.t b/t/op/pack.t
    index cf0e286509..bb9f865091 100644
    --- a/t/op/pack.t
    +++ b/t/op/pack.t
    @@ -12,7 +12,7 @@ my $no_endianness = $] > 5.009 ? '' :
     my $no_signedness = $] > 5.009 ? '' :
       "Signed/unsigned pack modifiers not available on this perl";
     
    -plan tests => 14717;
    +plan tests => 14718;
     
     use strict;
     use warnings qw(FATAL all);
    @@ -2081,3 +2081,10 @@ SKIP:
         fresh_perl_like('pack "c10f1073741824"', qr/Out of memory during pack/, { stderr => 1 },
     		    "integer overflow calculating allocation (multiply)");
     }
    +
    +{
    +    # [perl #132655] heap-buffer-overflow READ of size 11
    +    # only expect failure under ASAN (and maybe valgrind)
    +    fresh_perl_is('0.0 + unpack("u", "ab")', "", { stderr => 1 },
    +                  "ensure unpack u of invalid data nul terminates result");
    +}
    -- 
    2.11.0
    
    
  15. p5pRT commented on Aug 21, 2018

    @p5pRT
    Author

    From hanno@hboeck.de

    Hi,

    An out of bounds heap read can be triggered in perl with a call to
    unpack​:

    perl -e 'unpack("u/X", "r0");'

    It will read 11 bytes beyond a defined buffer.

    This can be detected by compiling perl with address sanitizer. Works
    with latest perl5 git code. I'm attaching a stack trace from address
    sanitizer below.


    ==26194==ERROR​: AddressSanitizer​: heap-buffer-overflow on address 0x60200000165a at pc 0x00000044b1fb bp 0x7ffedc131b90 sp 0x7ffedc131338
    READ of size 11 at 0x60200000165a thread T0
      #0 0x44b1fa in __interceptor_strlen (/r/perl/perl+0x44b1fa)
      #1 0xb2e253 in Perl_my_atof3 /f/perl/perl/numeric.c​:1412​:30
      #2 0xb2dc39 in Perl_my_atof /f/perl/perl/numeric.c​:1271​:13
      #3 0x892311 in S_sv_setnv /f/perl/perl/sv.c​:2112​:9
      #4 0x892311 in S_sv_2iuv_common /f/perl/perl/sv.c​:2314
      #5 0x88fced in Perl_sv_2iv_flags /f/perl/perl/sv.c​:2507​:6
      #6 0xb55726 in S_unpack_rec /f/perl/perl/pp_pack.c​:1828​:23
      #7 0xb42c24 in Perl_unpackstring /f/perl/perl/pp_pack.c​:851​:12
      #8 0xb5b012 in Perl_pp_unpack /f/perl/perl/pp_pack.c​:1861​:11
      #9 0x8498e7 in Perl_runops_standard /f/perl/perl/run.c​:41​:26
      #10 0x5b9804 in S_run_body /f/perl/perl/perl.c
      #11 0x5b8633 in perl_run /f/perl/perl/perl.c​:2611​:2
      #12 0x5076fe in main /f/perl/perl/perlmain.c​:122​:9
      #13 0x7f9376bcaae6 in __libc_start_main (/lib64/libc.so.6+0x21ae6)
      #14 0x436c89 in _start (/r/perl/perl+0x436c89)

    0x60200000165a is located 0 bytes to the right of 10-byte region [0x602000001650,0x60200000165a)
    allocated by thread T0 here​:
      #0 0x4d9723 in malloc (/r/perl/perl+0x4d9723)
      #1 0x7b3f14 in Perl_safesysmalloc /f/perl/perl/util.c​:153​:21
      #2 0x8891f3 in Perl_sv_grow /f/perl/perl/sv.c​:1600​:17
      #3 0x8b2b1a in Perl_newSV /f/perl/perl/sv.c​:5601​:2
      #4 0xb472cd in S_unpack_rec /f/perl/perl/pp_pack.c​:1729​:8
      #5 0xb42c24 in Perl_unpackstring /f/perl/perl/pp_pack.c​:851​:12
      #6 0xb5b012 in Perl_pp_unpack /f/perl/perl/pp_pack.c​:1861​:11
      #7 0x8498e7 in Perl_runops_standard /f/perl/perl/run.c​:41​:26
      #8 0x5b9804 in S_run_body /f/perl/perl/perl.c
      #9 0x5b8633 in perl_run /f/perl/perl/perl.c​:2611​:2
      #10 0x5076fe in main /f/perl/perl/perlmain.c​:122​:9
      #11 0x7f9376bcaae6 in __libc_start_main (/lib64/libc.so.6+0x21ae6)
      #12 0x436c89 in _start (/r/perl/perl+0x436c89)

    SUMMARY​: AddressSanitizer​: heap-buffer-overflow (/r/perl/perl+0x44b1fa) in __interceptor_strlen
    Shadow bytes around the buggy address​:
      0x0c047fff8270​: fa fa 06 fa fa fa 00 fa fa fa 00 02 fa fa 00 fa
      0x0c047fff8280​: fa fa 00 05 fa fa 00 00 fa fa 00 02 fa fa 00 00
      0x0c047fff8290​: fa fa 05 fa fa fa 00 01 fa fa 00 fa fa fa 00 06
      0x0c047fff82a0​: fa fa 00 01 fa fa 00 02 fa fa 02 fa fa fa fd fd
      0x0c047fff82b0​: fa fa fd fd fa fa 00 02 fa fa fd fa fa fa 00 02
    =>0x0c047fff82c0​: fa fa fd fa fa fa 00 02 fa fa 00[02]fa fa fa fa
      0x0c047fff82d0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff82e0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff82f0​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff8300​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
      0x0c047fff8310​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
    Shadow byte legend (one shadow byte represents 8 application bytes)​:
      Addressable​: 00
      Partially addressable​: 01 02 03 04 05 06 07
      Heap left redzone​: fa
      Freed heap region​: fd
      Stack left redzone​: f1
      Stack mid redzone​: f2
      Stack right redzone​: f3
      Stack after return​: f5
      Stack use after scope​: f8
      Global redzone​: f9
      Global init order​: f6
      Poisoned by user​: f7
      Container overflow​: fc
      Array cookie​: ac
      Intra object redzone​: bb
      ASan internal​: fe
      Left alloca redzone​: ca
      Right alloca redzone​: cb
    ==26194==ABORTING

    --
    Hanno Böck
    https://hboeck.de/

    mail/jabber​: hanno@​hboeck.de
    GPG​: FE73757FA60E4E21B937579FA5880072BBB51E42

  16. p5pRT commented on Sep 21, 2018

    @p5pRT
    Author

    From @tonycoz

    On Sun, 19 Aug 2018 23​:32​:55 -0700, tonyc wrote​:

    On Mon, 09 Apr 2018 08​:53​:03 -0700, davem wrote​:

    On Mon, Jan 22, 2018 at 09​:14​:09PM -0800, Tony Cook via RT wrote​:

    The problem here is the "u" decoder, which creates a new SV
    (upgraded
    to SVt_PV in the case that matters) and sets POK on it.

    When the "ab" fails to decode, no changes are made to the PV,
    leaving
    it unterminated.
    This should only cause perl to crash, there's nothing that will
    write
    to the memory block.

    This issue can only occur if the first byte in the string is not a
    uuencoding character - so no decoded data is emitted, so the
    attacker
    has zero control over what ends up the SV that's causing the
    problem.

    In the past we've treated similar issues as *not* being security
    issues.

    I don't think this is a security issue.

    Agreed. I think you should go ahead apply the patch.

    Here's the patch, I'll apply it in a couple of days, unless someone
    suggests we make this a security issue.

    More than a couple of days.

    Applied as 12cad9b. This ticket is now public.

    Tony

  17. p5pRT commented on Sep 21, 2018

    @p5pRT
    Author

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

  18. p5pRT commented on Sep 21, 2018

    @p5pRT
    Author

    From @tonycoz

    On Tue, 21 Aug 2018 04​:19​:15 -0700, hanno@​hboeck.de wrote​:

    Hi,

    An out of bounds heap read can be triggered in perl with a call to
    unpack​:

    perl -e 'unpack("u/X", "r0");'

    It will read 11 bytes beyond a defined buffer.

    This can be detected by compiling perl with address sanitizer. Works
    with latest perl5 git code. I'm attaching a stack trace from address
    sanitizer below.

    This is a duplicate of 132655, and the fix, 12cad9b, also fixes this.

    Tony

  19. p5pRT commented on Sep 21, 2018

    @p5pRT
    Author

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

  20. p5pRT commented on Sep 21, 2018

    @p5pRT
    Author

    From @tonycoz

    On Sun, 19 Aug 2018 22​:18​:33 -0700, tonyc wrote​:

    I believe this is the same issue as #132655.

    The fix for 132655 also fixes this, merging into 132655.

    Tony

  21. p5pRT commented on May 22, 2019

    @p5pRT
    Author

    From @khwilliamson

    Thank you for filing this report. You have helped make Perl better.

    With the release today of Perl 5.30.0, this and 160 other issues have been
    resolved.

    Perl 5.30.0 may be downloaded via​:
    https://metacpan.org/release/XSAWYERX/perl-5.30.0

    If you find that the problem persists, feel free to reopen this ticket.

  22. p5pRT commented on May 22, 2019

    @p5pRT
    Author

    @khwilliamson - Status changed from 'pending release' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions