fix(common): fix memory corruption for dynamic to static array conversion #1598
Conversation
🦋 Changeset detectedLatest commit: 360be09 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| // (without the length check this could lead to memory corruption) | ||
| assembly { | ||
| _result := add(_value, 0x20) | ||
| } |
There was a problem hiding this comment.
any way we can add a regression test for this to make sure we don't run into this in the future?
There was a problem hiding this comment.
memory corruption tests are tricky, I think I'm too sleepy to write 1 rn, we can add an issue for later, tests aren't production code so does freeze affect em?
There was a problem hiding this comment.
we can always add more tests later! planning to do extend the test suite in general next week
| _result := add(_value, 0x20) | ||
| if (_value.length < 1) { | ||
| // ignore invalid dynamic arrays that are too small | ||
| return _result; |
There was a problem hiding this comment.
if this is invalid, should this revert instead of returning?
There was a problem hiding this comment.
a 0-length array is kinda valid (it's uninitialized), so maybe I should change the comment?
and other cases are weird and shouldn't happen, but it's cheaper to return empty than add more checks
packages/common/src/codegen/render-solidity/renderTypeHelpers.ts
Outdated
Show resolved
Hide resolved
alvrs
force-pushed
the
dk1a/fix-memory-corruption
branch
from
f2144fd to
7739767
Compare
fixes #1005