solidity icon indicating copy to clipboard operation
solidity copied to clipboard

zero-length array types in expression context do not raise type errors

Open bshastry opened this issue 3 years ago • 14 comments
trafficstars

contract C {
  function f() public {
    abi.decode("",  (int[0][]));
  }
}

throws

https://github.com/ethereum/solidity/blob/84cdcec2cfb1fe9d4a9171d3ed0ffefd6107ee42/libsolidity/codegen/ABIFunctions.cpp#L1174

Repro

$ solc --ir test.sol

bshastry avatar Oct 25 '22 13:10 bshastry

@bshastry what is the priority of this bug (High, Medium, Low)?

NunoFilipeSantos avatar Oct 26 '22 13:10 NunoFilipeSantos

I would say Low because it is unlikely a legitimate user would try to decode an empty string into a zero length array.

bshastry avatar Oct 26 '22 14:10 bshastry

Output:

/solidity/libsolidity/codegen/ABIFunctions.cpp(1174): Throw in function std::string solidity::frontend::ABIFunctions::abiDecodingFunctionArrayAvailableLength(const solidity::frontend::ArrayType&, bool)
Dynamic exception type: boost::wrapexcept<solidity::langutil::InternalCompilerError>
std::exception::what: Solidity assertion failed
[solidity::util::tag_comment*] = Solidity assertion failed

This started happening in 0.8.8.

The [0] part seems important here. I think it should actually be a type error since int[0] is not a valid type (i.e. you cannot have a variable of this type). So given that, impact would be indeed low - this code would not work even if it wasn't causing an ICE.

But there's another case that's a bit more worrying:

contract C {
    function f() public {
        abi.encode(abi.decode("", (int[0])));
    }
}

This compiles just fine and it shouldn't. You probably cannot assign the result of that decoding to anything (because it's of type that does not exist) but as you can see above you can e.g. encode it again. This has a higher impact but it's still pretty unlikely to happen in practice other than by mistake so still not high enough for medium.

On the other hand I'd say that the effort is low so it's worth fixing. I think it only needs an extra check at the analysis stage. Cases that do the wrong thing without reporting an error are dangerous. Technically, it could classify as a codegen error.

cameel avatar Oct 26 '22 16:10 cameel

Hi @cameel I would like to get started with this issue can you give me a brief overview of how to get started with this

Krish-bhardwaj avatar Oct 28 '22 11:10 Krish-bhardwaj

@Krish-bhardwaj Take this example:

contract C {
    int[0] x;
}

It currently produces an error:

Error: Array with zero length specified.
 --> test.sol:2:9:
  |
2 |     int[0] x;
  |         ^

The example from this issue should be reporting the same error.

The error is reported by DeclarationTypeChecker::endVisit(ArrayTypeName). Probably one of the conditions in that function makes it skip checking abi.decode(). Pass the test case from the issue though solc and using a debugger find out why exactly the check is skipped. Then modify the code so that it's no longer skipped. Note that this will likely make other things break so you'll need to find a solution that keeps them working while also solving the problem.

Also, try to explore similar cases. When you fix it, try using zero-length arrays in various ways and make sure none of them is broken the same way abi.decode() is now. Try various places where it's possible to use a type (using for, type(), UDVT declarations, parameter lists, error/event declarations, function pointer arguments, type tuples, struct fields and anything else you can think of). Also try wrapping the zero-length part in various ways - e.g. like in this issue it's a part of a dynamic array, which is probably part of the reason why it's not detected. Add the cases you try as new test cases, even if they don't crash the compiler (unless we already have them). They're still useful as regression tests and overall for better coverage of features.

cameel avatar Oct 28 '22 12:10 cameel

@cameel before this i have never worked on developing compiler can you assist me how should i set up this C++ project !

i have read through your docs but i don't have much of clarity on the project set up .

Krish-bhardwaj avatar Oct 28 '22 13:10 Krish-bhardwaj

@Krish-bhardwaj I'm off this week, but if you need help please come to the #solidity-dev channel, you'll always find someone from the team there. Setting up for development is mostly a matter of cloning the repo, installing the dependencies and making sure you can run tests. This is covered by the Contributing page, but if something is missing, let us know (or just open an issue :)).

cameel avatar Nov 03 '22 14:11 cameel

i would like to contribute to this issue , can you let me know how i can solve it.

redone9211 avatar Jan 22 '23 05:01 redone9211

@redone9211 See https://github.com/ethereum/solidity/issues/13652#issuecomment-1294913283.

cameel avatar Jan 22 '23 19:01 cameel

Hi, I would like to work on this issue. Please assign.

ananyak19 avatar Jan 23 '23 07:01 ananyak19

Hey! I would really love to work on this issue.

yoharsh14 avatar Mar 19 '23 12:03 yoharsh14

Feel tree to try. https://github.com/ethereum/solidity/issues/13652#issuecomment-1294913283 should get you started.

cameel avatar Mar 20 '23 09:03 cameel

Hey @cameel definitely love to make this issue my first contri. Can I contribute?

shikhar2712 avatar Mar 23 '23 10:03 shikhar2712

Hi all. I've remove the good first issue label from here, since this isn't really a good first issue. In addition, if this is your first time contributing to the Solidity project, try to find another issue that is more appropriate. Of course, feel leave a message on the issue you'd like to work on to ask whether it's a good fit, since this will make things a lot easier for everyone involved in the long run.

nikola-matic avatar Mar 27 '23 10:03 nikola-matic