Neo: JSON's syscalls can't deal with NULL values

[shargon]

Describe the bug

In Red4sec we are preparing a fuzzing process for different neo areas. We find a issue with null values.

To Reproduce

using (var script = new ScriptBuilder())
            using (var engine = new ApplicationEngine(TriggerType.Application, null, null, 0, true))
            {
                script.EmitPush("{\"a\":1,\"b\":null}");
                script.EmitSysCall(InteropService.Neo_Json_Deserialize);
                script.EmitSysCall(InteropService.Neo_Json_Serialize);
                engine.LoadScript(script.ToArray());
                engine.Execute();
                var ret = engine.ResultStack.Peek().GetString();
            }

Expected behavior

The expected result should be {"a":1,"b":null}

Screenshots

Source Reference:
https://github.com/neo-project/neo/blob/0a4f7806eb41552c733beb2cfda5c59c12eb095b/neo/SmartContract/JsonSerializer.cs#L101

Platform:

• Version [e.g. neo 3x]

Additional context
In addition, it should be possible to reverse a null value, to a null json value, and the VM will treat null values ​​as a byte [0], how will it be possible to distinguish between an empty byte array and a null?

[erikzhang]

Should we add new opcodes PUSHNULL and ISNULL?

[shargon]

I think that we can use ByteArray, but a special one, we have a null item.

https://github.com/neo-project/neo-vm/blob/6676592b667cd39c26f46af41d7484cd054e4da1/src/neo-vm/StackItem.cs#L13

Maybe we can check with ReferenceEqual on PUSHNULL and ISNULL?

[erikzhang]

How to distinguish between null and byte[0]?

[shargon]

var item=stack.Pop();
if (Object.ReferenceEquals(item ,StackItem.Null)

[erikzhang]

Please check neo-project/neo-vm#208

[igormcoelho]

How to distinguish between null and byte[0]?

new ByteArray(null) could have a constructor that receives null and indicates this in a local flag. Other possibility is use Interop stack item for Json (that supports null naturally).

[UPDATE] I tried this approach, but went worse than having another stackitem. Reason is that it brought incertainty to ByteArray internals, which is not good (at all). I'm supporting new stackitem Null now.