From e6910e81d2e2ad539f7cbe51924f03f52b3d882b Mon Sep 17 00:00:00 2001 From: ergl Date: Tue, 6 Oct 2020 23:42:58 +0200 Subject: [PATCH 1/4] Prevent Array.from_cpointer from accepting a null pointer --- packages/builtin/array.pony | 16 +++++++++++----- packages/builtin_test/_test.pony | 22 ++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/packages/builtin/array.pony b/packages/builtin/array.pony index 1b1f75cf62..e2f1cdbc84 100644 --- a/packages/builtin/array.pony +++ b/packages/builtin/array.pony @@ -128,13 +128,19 @@ class Array[A] is Seq[A] """ _size = len - if alloc > len then - _alloc = alloc + if ptr.is_null() then + _size = 0 + _alloc = 1 + _ptr = Pointer[A]._alloc(_alloc) else - _alloc = len - end + if alloc > len then + _alloc = alloc + else + _alloc = len + end - _ptr = ptr + _ptr = ptr + end fun _copy_to( ptr: Pointer[this->A!], diff --git a/packages/builtin_test/_test.pony b/packages/builtin_test/_test.pony index b1278f3109..f6afe85b40 100644 --- a/packages/builtin_test/_test.pony +++ b/packages/builtin_test/_test.pony @@ -54,6 +54,7 @@ actor Main is TestList test(_TestStringUnchop) test(_TestStringRepeatStr) test(_TestStringConcatOffsetLen) + test(_TestStringFromCPointer) test(_TestSpecialValuesF32) test(_TestSpecialValuesF64) test(_TestArrayAppend) @@ -68,6 +69,7 @@ actor Main is TestList test(_TestArraySwapElements) test(_TestArrayChop) test(_TestArrayUnchop) + test(_TestArrayFromCPointer) test(_TestMath128) test(_TestRem) test(_TestMod) @@ -1275,6 +1277,16 @@ class iso _TestStringConcatOffsetLen is UnitTest fun apply(h: TestHelper) => h.assert_eq[String](recover String.>concat("ABCD".values(), 1, 2) end, "BC") +class iso _TestStringFromCPointer is UnitTest + """ + Test creating a string from a pointer + """ + fun name(): String => "builtin/String.from_cpointer" + + fun apply(h: TestHelper) => + let str = String.from_cpointer(Pointer[U8], 1, 1) + h.assert_eq[USize](0, str.size()) + class iso _TestArrayAppend is UnitTest fun name(): String => "builtin/Array.append" @@ -1681,6 +1693,16 @@ class iso _TestArrayUnchop is UnitTest error end +class iso _TestArrayFromCPointer is UnitTest + """ + Test creating an array from a pointer + """ + fun name(): String => "builtin/Array.from_cpointer" + + fun apply(h: TestHelper) => + let arr = Array[U8].from_cpointer(Pointer[U8], 1, 1) + h.assert_eq[USize](0, arr.size()) + class iso _TestMath128 is UnitTest """ Test 128 bit integer math. From b226dab7de9556f336d6aab1214f210ef04fed16 Mon Sep 17 00:00:00 2001 From: ergl Date: Wed, 7 Oct 2020 09:29:45 +0200 Subject: [PATCH 2/4] Fix extra allocation on zero-lenght arrays when using from_cpointer --- packages/builtin/array.pony | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/builtin/array.pony b/packages/builtin/array.pony index e2f1cdbc84..4adac6e819 100644 --- a/packages/builtin/array.pony +++ b/packages/builtin/array.pony @@ -126,19 +126,17 @@ class Array[A] is Seq[A] Create an array from a C-style pointer and length. The contents are not copied. """ - _size = len - if ptr.is_null() then _size = 0 - _alloc = 1 - _ptr = Pointer[A]._alloc(_alloc) + _alloc = 0 + _ptr = ptr else + _size = len if alloc > len then _alloc = alloc else _alloc = len end - _ptr = ptr end From 0fdc95a0cd8e9d7bc32575a7aa7201d66f36abe7 Mon Sep 17 00:00:00 2001 From: ergl Date: Wed, 7 Oct 2020 09:46:56 +0200 Subject: [PATCH 3/4] Add release notes [skip ci] --- .release-notes/3675.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .release-notes/3675.md diff --git a/.release-notes/3675.md b/.release-notes/3675.md new file mode 100644 index 0000000000..a2be2759a1 --- /dev/null +++ b/.release-notes/3675.md @@ -0,0 +1,7 @@ +## Fix memory safety problem with Array.from_cpointer + +Previously, the `from_cpointer` method in arrays would trust the user to pass a +valid pointer. This created an issue where the user could pass a null pointer using +`Pointer.create`, leading to a situation where memory safety could be violated +by trying to access an element of the array. This change makes it safe to pass a +null pointer to `from_cpointer`, which will create a zero-length array. From e40f86c4a8c369b56642059535869bb367a2715d Mon Sep 17 00:00:00 2001 From: ergl Date: Wed, 7 Oct 2020 21:57:50 +0200 Subject: [PATCH 4/4] Cosmetic changes --- .release-notes/3675.md | 6 +----- packages/builtin/array.pony | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.release-notes/3675.md b/.release-notes/3675.md index a2be2759a1..100657a487 100644 --- a/.release-notes/3675.md +++ b/.release-notes/3675.md @@ -1,7 +1,3 @@ ## Fix memory safety problem with Array.from_cpointer -Previously, the `from_cpointer` method in arrays would trust the user to pass a -valid pointer. This created an issue where the user could pass a null pointer using -`Pointer.create`, leading to a situation where memory safety could be violated -by trying to access an element of the array. This change makes it safe to pass a -null pointer to `from_cpointer`, which will create a zero-length array. +Previously, the `from_cpointer` method in arrays would trust the user to pass a valid pointer. This created an issue where the user could pass a null pointer using `Pointer.create`, leading to a situation where memory safety could be violated by trying to access an element of the array. This change makes it safe to pass a null pointer to `from_cpointer`, which will create a zero-length array. diff --git a/packages/builtin/array.pony b/packages/builtin/array.pony index 4adac6e819..b526c7b9b1 100644 --- a/packages/builtin/array.pony +++ b/packages/builtin/array.pony @@ -129,7 +129,6 @@ class Array[A] is Seq[A] if ptr.is_null() then _size = 0 _alloc = 0 - _ptr = ptr else _size = len if alloc > len then @@ -137,9 +136,10 @@ class Array[A] is Seq[A] else _alloc = len end - _ptr = ptr end + _ptr = ptr + fun _copy_to( ptr: Pointer[this->A!], copy_len: USize,