stack size can exceed MaxStackSize #3300
Comments
|
It's very dangerous if someone can combine this with some O(n2) DoS attacks like what have been shown by @vang1ong7ang in the past. |
This was referenced Jun 7, 2024
|
it breaks a fundamental assumption of neovm and it should be fixed soon and published |
Merged
14 tasks
shargon
added
Bug
|
With careful construction, it can theoretically generate an array containing at least 10 million data which may cost ~10 hours. Within 20 GAS. |
AnnaShaleva
added a commit
to nspcc-dev/neo-go
that referenced
this issue
Jun 11, 2024
This test ensures that NeoGo node doesn't have the DeepCopy problem described in neo-project/neo#3300 and fixed in neo-project/neo#3301. This problem leads to the fact that Notifications items are not being properly refcounted by C# node which leads to possibility to build an enormously large object on stack. Go node doesn't have this problem. The reason (at least, as I understand it) is in the fact that C# node performs objects refcounting inside the DeepCopy even if the object itself is not yet on stack. I.e. System.Runtime.Notify handler immediately adds references to the notification argumetns inside DeepCopy: https://github.com/neo-project/neo/blob/b1d27f0189861167b8005a7e40e6d8500fb48cc4/src/Neo.VM/Types/Array.cs#L108 https://github.com/neo-project/neo/blob/b1d27f0189861167b8005a7e40e6d8500fb48cc4/src/Neo.VM/Types/Array.cs#L75 Whereas Go node just performs the honest DeepCopy without references counting: https://github.com/nspcc-dev/neo-go/blob/b66cea5cccbcc8446312b012e29aaf2d1837430a/pkg/vm/stackitem/item.go#L1223 Going further, C# node clears refs for notification arguments (for array and underlying array items). System.Runtime.GetNotifications pushes the notificaiton args array back on stack and increments counter only for the external array, not for its arguments. Which results in negative refcounter once notificaiton is removed from the stack. The fix itself (https://github.com/Jim8y/neo/blob/f471c0542ddd8f527478fbdcda76a3ab9194b958/src/Neo/SmartContract/NotifyEventArgs.cs#L84) doesn't need to be ported to NeoGo because Go node adds object to the refcounter only at the moment when it's being pushed to stack by System.Runtime.GetNotifications handler. This object is treated as new object since it was deepcopied earlier by System.Runtime.Notify handler: https://github.com/nspcc-dev/neo-go/blob/b66cea5cccbcc8446312b012e29aaf2d1837430a/pkg/vm/stack.go#L178. Thus, no functoinal changes from the NeoGo side. And we won't intentionally break our node to follow C# pre-Domovoi invalid behaviour. Close #3484, close #3482. Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Describe the bug
You can make a large object larger than the limit
MaxStackSize.To Reproduce
Expected behavior
FAULT
Screenshots
Platform:
(Optional) Additional context
I believe that
Runtime.Notifyshould be blamed.The text was updated successfully, but these errors were encountered: