|
@@ -0,0 +1,183 @@
|
|
|
+From 4bed614e707c0644c06e117f848fa12605c711cd Mon Sep 17 00:00:00 2001
|
|
|
+From: Angus Gratton <angus@redyak.com.au>
|
|
|
+Date: Tue, 13 Feb 2024 09:24:36 +1100
|
|
|
+Subject: [PATCH] py/objarray: Fix use-after-free if extending a bytearray from
|
|
|
+ itself.
|
|
|
+
|
|
|
+Two cases, one assigning to a slice.
|
|
|
+Closes https://github.com/micropython/micropython/issues/13283
|
|
|
+
|
|
|
+Second is extending a slice from itself, similar logic.
|
|
|
+
|
|
|
+In both cases the problem occurs when m_renew causes realloc to move the
|
|
|
+buffer, leaving a dangling pointer behind.
|
|
|
+
|
|
|
+There are more complex and hard to fix cases when either argument is a
|
|
|
+memoryview into the buffer, currently resizing to a new address breaks
|
|
|
+memoryviews into that object.
|
|
|
+
|
|
|
+Reproducing this bug and confirming the fix was done by running the unix
|
|
|
+port under valgrind with GC-aware extensions.
|
|
|
+
|
|
|
+Note in default configurations with GIL this bug exists but has no impact
|
|
|
+(the free buffer won't be reused while the function is still executing, and
|
|
|
+is no longer referenced after it returns).
|
|
|
+
|
|
|
+Signed-off-by: Angus Gratton <angus@redyak.com.au>
|
|
|
+CVE: CVE-2024-8947
|
|
|
+Upstream: https://github.com/micropython/micropython/commit/4bed614e707c0644c06e117f848fa12605c711cd
|
|
|
+Signed-off-by: Thomas Perale <thomas.perale@mind.be>
|
|
|
+---
|
|
|
+ py/objarray.c | 20 ++++++++++++++++----
|
|
|
+ tests/basics/bytearray_add.py | 9 ++++++++-
|
|
|
+ tests/basics/bytearray_add_self.py | 8 ++++++++
|
|
|
+ tests/basics/bytearray_add_self.py.exp | 1 +
|
|
|
+ tests/basics/bytearray_slice_assign.py | 18 ++++++++++++------
|
|
|
+ 5 files changed, 45 insertions(+), 11 deletions(-)
|
|
|
+ create mode 100644 tests/basics/bytearray_add_self.py
|
|
|
+ create mode 100644 tests/basics/bytearray_add_self.py.exp
|
|
|
+
|
|
|
+diff --git a/py/objarray.c b/py/objarray.c
|
|
|
+index 1fff234822521..803af2cd270c7 100644
|
|
|
+--- a/py/objarray.c
|
|
|
++++ b/py/objarray.c
|
|
|
+@@ -424,6 +424,13 @@ static mp_obj_t array_extend(mp_obj_t self_in, mp_obj_t arg_in) {
|
|
|
+ if (self->free < len) {
|
|
|
+ self->items = m_renew(byte, self->items, (self->len + self->free) * sz, (self->len + len) * sz);
|
|
|
+ self->free = 0;
|
|
|
++
|
|
|
++ if (self_in == arg_in) {
|
|
|
++ // Get arg_bufinfo again in case self->items has moved
|
|
|
++ //
|
|
|
++ // (Note not possible to handle case that arg_in is a memoryview into self)
|
|
|
++ mp_get_buffer_raise(arg_in, &arg_bufinfo, MP_BUFFER_READ);
|
|
|
++ }
|
|
|
+ } else {
|
|
|
+ self->free -= len;
|
|
|
+ }
|
|
|
+@@ -456,7 +463,8 @@ static mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
|
|
|
+ #if MICROPY_PY_ARRAY_SLICE_ASSIGN
|
|
|
+ // Assign
|
|
|
+ size_t src_len;
|
|
|
+- void *src_items;
|
|
|
++ uint8_t *src_items;
|
|
|
++ size_t src_offs = 0;
|
|
|
+ size_t item_sz = mp_binary_get_size('@', o->typecode & TYPECODE_MASK, NULL);
|
|
|
+ if (mp_obj_is_obj(value) && MP_OBJ_TYPE_GET_SLOT_OR_NULL(((mp_obj_base_t *)MP_OBJ_TO_PTR(value))->type, subscr) == array_subscr) {
|
|
|
+ // value is array, bytearray or memoryview
|
|
|
+@@ -469,7 +477,7 @@ static mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
|
|
|
+ src_items = src_slice->items;
|
|
|
+ #if MICROPY_PY_BUILTINS_MEMORYVIEW
|
|
|
+ if (mp_obj_is_type(value, &mp_type_memoryview)) {
|
|
|
+- src_items = (uint8_t *)src_items + (src_slice->memview_offset * item_sz);
|
|
|
++ src_offs = src_slice->memview_offset * item_sz;
|
|
|
+ }
|
|
|
+ #endif
|
|
|
+ } else if (mp_obj_is_type(value, &mp_type_bytes)) {
|
|
|
+@@ -504,13 +512,17 @@ static mp_obj_t array_subscr(mp_obj_t self_in, mp_obj_t index_in, mp_obj_t value
|
|
|
+ // TODO: alloc policy; at the moment we go conservative
|
|
|
+ o->items = m_renew(byte, o->items, (o->len + o->free) * item_sz, (o->len + len_adj) * item_sz);
|
|
|
+ o->free = len_adj;
|
|
|
++ // m_renew may have moved o->items
|
|
|
++ if (src_items == dest_items) {
|
|
|
++ src_items = o->items;
|
|
|
++ }
|
|
|
+ dest_items = o->items;
|
|
|
+ }
|
|
|
+ mp_seq_replace_slice_grow_inplace(dest_items, o->len,
|
|
|
+- slice.start, slice.stop, src_items, src_len, len_adj, item_sz);
|
|
|
++ slice.start, slice.stop, src_items + src_offs, src_len, len_adj, item_sz);
|
|
|
+ } else {
|
|
|
+ mp_seq_replace_slice_no_grow(dest_items, o->len,
|
|
|
+- slice.start, slice.stop, src_items, src_len, item_sz);
|
|
|
++ slice.start, slice.stop, src_items + src_offs, src_len, item_sz);
|
|
|
+ // Clear "freed" elements at the end of list
|
|
|
+ // TODO: This is actually only needed for typecode=='O'
|
|
|
+ mp_seq_clear(dest_items, o->len + len_adj, o->len, item_sz);
|
|
|
+diff --git a/tests/basics/bytearray_add.py b/tests/basics/bytearray_add.py
|
|
|
+index a7e2b57374255..1f30a3b740e95 100644
|
|
|
+--- a/tests/basics/bytearray_add.py
|
|
|
++++ b/tests/basics/bytearray_add.py
|
|
|
+@@ -15,4 +15,11 @@
|
|
|
+
|
|
|
+ # this inplace add tests the code when the buffer doesn't need to be increased
|
|
|
+ b = bytearray()
|
|
|
+-b += b''
|
|
|
++b += b""
|
|
|
++
|
|
|
++# extend a bytearray from itself
|
|
|
++b = bytearray(b"abcdefgh")
|
|
|
++for _ in range(4):
|
|
|
++ c = bytearray(b) # extra allocation, as above
|
|
|
++ b.extend(b)
|
|
|
++print(b)
|
|
|
+diff --git a/tests/basics/bytearray_add_self.py b/tests/basics/bytearray_add_self.py
|
|
|
+new file mode 100644
|
|
|
+index 0000000000000..94ae8689fd16c
|
|
|
+--- /dev/null
|
|
|
++++ b/tests/basics/bytearray_add_self.py
|
|
|
+@@ -0,0 +1,8 @@
|
|
|
++# add a bytearray to itself
|
|
|
++# This is not supported by CPython as of 3.11.18.
|
|
|
++
|
|
|
++b = bytearray(b"123456789")
|
|
|
++for _ in range(4):
|
|
|
++ c = bytearray(b) # extra allocation increases chance 'b' has to relocate
|
|
|
++ b += b
|
|
|
++print(b)
|
|
|
+diff --git a/tests/basics/bytearray_add_self.py.exp b/tests/basics/bytearray_add_self.py.exp
|
|
|
+new file mode 100644
|
|
|
+index 0000000000000..5ef948157ac0f
|
|
|
+--- /dev/null
|
|
|
++++ b/tests/basics/bytearray_add_self.py.exp
|
|
|
+@@ -0,0 +1 @@
|
|
|
++bytearray(b'123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789123456789')
|
|
|
+diff --git a/tests/basics/bytearray_slice_assign.py b/tests/basics/bytearray_slice_assign.py
|
|
|
+index fa7878e10ddbb..4de0819042a13 100644
|
|
|
+--- a/tests/basics/bytearray_slice_assign.py
|
|
|
++++ b/tests/basics/bytearray_slice_assign.py
|
|
|
+@@ -18,7 +18,7 @@
|
|
|
+ l[1:3] = bytearray()
|
|
|
+ print(l)
|
|
|
+ l = bytearray(x)
|
|
|
+-#del l[1:3]
|
|
|
++# del l[1:3]
|
|
|
+ print(l)
|
|
|
+
|
|
|
+ l = bytearray(x)
|
|
|
+@@ -28,7 +28,7 @@
|
|
|
+ l[:3] = bytearray()
|
|
|
+ print(l)
|
|
|
+ l = bytearray(x)
|
|
|
+-#del l[:3]
|
|
|
++# del l[:3]
|
|
|
+ print(l)
|
|
|
+
|
|
|
+ l = bytearray(x)
|
|
|
+@@ -38,7 +38,7 @@
|
|
|
+ l[:-3] = bytearray()
|
|
|
+ print(l)
|
|
|
+ l = bytearray(x)
|
|
|
+-#del l[:-3]
|
|
|
++# del l[:-3]
|
|
|
+ print(l)
|
|
|
+
|
|
|
+ # slice assignment that extends the array
|
|
|
+@@ -61,8 +61,14 @@
|
|
|
+ print(b)
|
|
|
+
|
|
|
+ # Growth of bytearray via slice extension
|
|
|
+-b = bytearray(b'12345678')
|
|
|
+-b.append(57) # expand and add a bit of unused space at end of the bytearray
|
|
|
++b = bytearray(b"12345678")
|
|
|
++b.append(57) # expand and add a bit of unused space at end of the bytearray
|
|
|
+ for i in range(400):
|
|
|
+- b[-1:] = b'ab' # grow slowly into the unused space
|
|
|
++ b[-1:] = b"ab" # grow slowly into the unused space
|
|
|
++print(len(b), b)
|
|
|
++
|
|
|
++# Growth of bytearray via slice extension from itself
|
|
|
++b = bytearray(b"1234567")
|
|
|
++for i in range(3):
|
|
|
++ b[-1:] = b
|
|
|
+ print(len(b), b)
|