Brad Bishop | 220d553 | 2018-08-14 00:59:39 +0100 | [diff] [blame] | 1 | From f17fed5006177dce8ac48229c424a2da0d6ba492 Mon Sep 17 00:00:00 2001 |
| 2 | From: Tony Cook <tony@develop-help.com> |
| 3 | Date: Tue, 8 Aug 2017 09:32:58 +1000 |
| 4 | Subject: [PATCH] (perl #131844) fix various space calculation issues in |
| 5 | pp_pack.c |
| 6 | |
| 7 | - for the originally reported case, if the start/cur pointer is in the |
| 8 | top 75% of the address space the add (cur) + glen addition would |
| 9 | overflow, resulting in the condition failing incorrectly. |
| 10 | |
| 11 | - the addition of the existing space used to the space needed could |
| 12 | overflow, resulting in too small an allocation and a buffer overflow. |
| 13 | |
| 14 | - the scaling for UTF8 could overflow. |
| 15 | |
| 16 | - the multiply to calculate the space needed for many items could |
| 17 | overflow. |
| 18 | |
| 19 | For the first case, do a space calculation without making new pointers. |
| 20 | |
| 21 | For the other cases, detect the overflow and croak if there's an |
| 22 | overflow. |
| 23 | |
| 24 | Originally this used Size_t_MAX as the maximum size of a memory |
| 25 | allocation, but for -DDEBUGGING builds realloc() throws a panic for |
| 26 | allocations over half the address space in size, changing the error |
| 27 | reported for the allocation. |
| 28 | |
| 29 | For non-DEBUGGING builds the Size_t_MAX limit has the small chance |
| 30 | of finding a system that has 3GB of contiguous space available, and |
| 31 | allocating that space, which could be a denial of servce in some cases. |
| 32 | |
| 33 | Unfortunately changing the limit to half the address space means that |
| 34 | the exact case with the original issue can no longer occur, so the |
| 35 | test is no longer testing against the address + length issue that |
| 36 | caused the original problem, since the allocation is failing earlier. |
| 37 | |
| 38 | One option would be to change the test so the size request by pack is |
| 39 | just under 2GB, but this has a higher (but still low) probability that |
| 40 | the system has the address space available, and will actually try to |
| 41 | allocate the memory, so let's not do that. |
| 42 | |
| 43 | Note: changed |
| 44 | plan tests => 14713; |
| 45 | to |
| 46 | plan tests => 14712; |
| 47 | in a/t/op/pack.t |
| 48 | to apply this patch on perl 5.24.1. |
| 49 | |
| 50 | CVE: CVE-2018-6913 |
| 51 | Upstream-Status: Backport [https://perl5.git.perl.org/perl.git/commitdiff/f17fed5006177dce8ac48229c424a2da0d6ba492] |
| 52 | |
| 53 | Signed-off-by: Jagadeesh Krishnanjanappa <jkrishnanjanappa@mvista.com> |
| 54 | --- |
| 55 | pp_pack.c | 25 +++++++++++++++++++++---- |
| 56 | t/op/pack.t | 24 +++++++++++++++++++++++- |
| 57 | 2 files changed, 44 insertions(+), 5 deletions(-) |
| 58 | |
| 59 | diff --git a/pp_pack.c b/pp_pack.c |
| 60 | index 8937d6d715..5e9cc64301 100644 |
| 61 | --- a/pp_pack.c |
| 62 | +++ b/pp_pack.c |
| 63 | @@ -357,11 +357,28 @@ STMT_START { \ |
| 64 | } \ |
| 65 | } STMT_END |
| 66 | |
| 67 | +#define SAFE_UTF8_EXPAND(var) \ |
| 68 | +STMT_START { \ |
| 69 | + if ((var) > SSize_t_MAX / UTF8_EXPAND) \ |
| 70 | + Perl_croak(aTHX_ "%s", "Out of memory during pack()"); \ |
| 71 | + (var) = (var) * UTF8_EXPAND; \ |
| 72 | +} STMT_END |
| 73 | + |
| 74 | +#define GROWING2(utf8, cat, start, cur, item_size, item_count) \ |
| 75 | +STMT_START { \ |
| 76 | + if (SSize_t_MAX / (item_size) < (item_count)) \ |
| 77 | + Perl_croak(aTHX_ "%s", "Out of memory during pack()"); \ |
| 78 | + GROWING((utf8), (cat), (start), (cur), (item_size) * (item_count)); \ |
| 79 | +} STMT_END |
| 80 | + |
| 81 | #define GROWING(utf8, cat, start, cur, in_len) \ |
| 82 | STMT_START { \ |
| 83 | STRLEN glen = (in_len); \ |
| 84 | - if (utf8) glen *= UTF8_EXPAND; \ |
| 85 | - if ((cur) + glen >= (start) + SvLEN(cat)) { \ |
| 86 | + STRLEN catcur = (STRLEN)((cur) - (start)); \ |
| 87 | + if (utf8) SAFE_UTF8_EXPAND(glen); \ |
| 88 | + if (SSize_t_MAX - glen < catcur) \ |
| 89 | + Perl_croak(aTHX_ "%s", "Out of memory during pack()"); \ |
| 90 | + if (catcur + glen >= SvLEN(cat)) { \ |
| 91 | (start) = sv_exp_grow(cat, glen); \ |
| 92 | (cur) = (start) + SvCUR(cat); \ |
| 93 | } \ |
| 94 | @@ -372,7 +389,7 @@ STMT_START { \ |
| 95 | STMT_START { \ |
| 96 | const STRLEN glen = (in_len); \ |
| 97 | STRLEN gl = glen; \ |
| 98 | - if (utf8) gl *= UTF8_EXPAND; \ |
| 99 | + if (utf8) SAFE_UTF8_EXPAND(gl); \ |
| 100 | if ((cur) + gl >= (start) + SvLEN(cat)) { \ |
| 101 | *cur = '\0'; \ |
| 102 | SvCUR_set((cat), (cur) - (start)); \ |
| 103 | @@ -2126,7 +2143,7 @@ S_pack_rec(pTHX_ SV *cat, tempsym_t* sym |
| 104 | if (props && !(props & PACK_SIZE_UNPREDICTABLE)) { |
| 105 | /* We can process this letter. */ |
| 106 | STRLEN size = props & PACK_SIZE_MASK; |
| 107 | - GROWING(utf8, cat, start, cur, (STRLEN) len * size); |
| 108 | + GROWING2(utf8, cat, start, cur, size, (STRLEN)len); |
| 109 | } |
| 110 | } |
| 111 | |
| 112 | diff --git a/t/op/pack.t b/t/op/pack.t |
| 113 | index 664aaaf1b0..cf0e286509 100644 |
| 114 | --- a/t/op/pack.t |
| 115 | +++ b/t/op/pack.t |
| 116 | @@ -12,7 +12,7 @@ my $no_endianness = $] > 5.009 ? '' : |
| 117 | my $no_signedness = $] > 5.009 ? '' : |
| 118 | "Signed/unsigned pack modifiers not available on this perl"; |
| 119 | |
| 120 | -plan tests => 14712; |
| 121 | +plan tests => 14717; |
| 122 | |
| 123 | use strict; |
| 124 | use warnings qw(FATAL all); |
| 125 | @@ -2044,3 +2044,25 @@ ok(1, "argument underflow did not crash" |
| 126 | is(pack("H40", $up_nul), $twenty_nuls, |
| 127 | "check pack H zero fills (utf8 source)"); |
| 128 | } |
| 129 | + |
| 130 | +SKIP: |
| 131 | +{ |
| 132 | + # [perl #131844] pointer addition overflow |
| 133 | + $Config{ptrsize} == 4 |
| 134 | + or skip "[perl #131844] need 32-bit build for this test", 4; |
| 135 | + # prevent ASAN just crashing on the allocation failure |
| 136 | + local $ENV{ASAN_OPTIONS} = $ENV{ASAN_OPTIONS}; |
| 137 | + $ENV{ASAN_OPTIONS} .= ",allocator_may_return_null=1"; |
| 138 | + fresh_perl_like('pack "f999999999"', qr/Out of memory during pack/, { stderr => 1 }, |
| 139 | + "pointer addition overflow"); |
| 140 | + |
| 141 | + # integer (STRLEN) overflow from addition of glen to current length |
| 142 | + fresh_perl_like('pack "c10f1073741823"', qr/Out of memory during pack/, { stderr => 1 }, |
| 143 | + "integer overflow calculating allocation (addition)"); |
| 144 | + |
| 145 | + fresh_perl_like('pack "W10f536870913", 256', qr/Out of memory during pack/, { stderr => 1 }, |
| 146 | + "integer overflow calculating allocation (utf8)"); |
| 147 | + |
| 148 | + fresh_perl_like('pack "c10f1073741824"', qr/Out of memory during pack/, { stderr => 1 }, |
| 149 | + "integer overflow calculating allocation (multiply)"); |
| 150 | +} |
| 151 | -- |
| 152 | 2.15.1-424-g9478a660812 |
| 153 | |