ponyc: Memory corruption via Array.copy_to
There’s bugs in Array’s copy to where it doesn’t validate input
for example:
actor Main
new create(e: Env) =>
var src: Array[U8] = [1]
var dest: Array[U8] = [11; 1; 2; 3; 4; 5; 6]
try
e.out.print(dest(0)?.string())
end
src.copy_to(dest, 11, 0, 10)
try
e.out.print(dest(0)?.string())
end
prints 11 and then prints 0. copying from an out of index src_index
shouldn’t be changing the destination array, it should be an error.
To fix, copy_to needs to become partial.
As part of this change, the slice
method that use copy_to
be switched to using _copy_to
so that it isn’t required to become partial UNLESS, slice actually needs to be partial. We should validate that it doesn’t.
Some things that we should be adding copy_to
tests methods for:
- src index is outside of range (should be a bug now)
- dest index is outside of valid range (should be handled correctly now)
- length to copy goes outside of valid range (should be a bug now)
- array was never initialized (so _ptr is null) (see example-ish test in #4173)
- array was initialized but is now empty (_ptr isn’t null, but size is 0) (see example-ish test in #4173)
Here’s some example code that shows the length bug:
actor Main
new create(e: Env) =>
var src: Array[U8] = [1]
var dest: Array[U8] = [11; 1; 2; 3; 4; 5; 6]
try
e.out.print(dest(3)?.string())
end
src.copy_to(dest, 0, 0, 10)
try
e.out.print(dest(3)?.string())
end
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Comments: 21 (21 by maintainers)
Commits related to this issue
- Update array.pony Update fix to include #4174 — committed to stefandd/ponyc by stefandd 2 years ago
- Update _test.pony Include tests for #4174 — committed to stefandd/ponyc by stefandd 2 years ago
- Further update to array.pony to fix #4174 — committed to stefandd/ponyc by stefandd 2 years ago
- Fix memory corruption that can arise from Array.copy_to This is based heavily on stefandd's work from https://github.com/ponylang/ponyc/pull/4173 Closes #4174 — committed to ponylang/ponyc by SeanTAllen 5 months ago
- Fix memory corruption that can arise from Array.copy_to This is based heavily on stefandd's work from https://github.com/ponylang/ponyc/pull/4173 Closes #4174 — committed to ponylang/ponyc by SeanTAllen 5 months ago
- Fix memory corruption that can arise from Array.copy_to This is based heavily on stefandd's work from https://github.com/ponylang/ponyc/pull/4173 Closes #4174 — committed to ponylang/ponyc by SeanTAllen 5 months ago
- Fix memory corruption that can arise from Array.copy_to This is based heavily on stefandd's work from https://github.com/ponylang/ponyc/pull/4173 Closes #4174 — committed to ponylang/ponyc by SeanTAllen 5 months ago
- Fix memory corruption that can arise from Array.copy_to (#4490) This is based heavily on stefandd's work from https://github.com/ponylang/ponyc/pull/4173 Closes #4174 — committed to ponylang/ponyc by SeanTAllen 5 months ago
@jemc Your proposal sounds good. I came round on your idea: your point about having a silent breaking change was a good one.