ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.45k stars 5.8k forks source link

Yul object names with dots are accepted but ambiguous #15540

Open cameel opened 1 month ago

cameel commented 1 month ago

Description

An identifier like A.B.C is a valid name for a Yul object. However, the compiler also uses dots as separators in qualified object/data names. This creates ambiguity when names with dots are passed to builtins like dataoffset() or datasize().

Such names are interpreted as paths by some parts of the compiler and as names by other parts. This results in errors or ICEs unless both possibilities are defined. And when both are defined, it's possible that some checks are getting bypassed.

Environment

Steps to Reproduce

Unreferenced object

unreferenced.yul

object "A.B.C" {
    code {}
}
solc --strict-assembly unreferenced.yul --asm --debug-info none
======= unreferenced.yul (EVM) =======

Text representation:
  stop

Dots in names are accepted and apparently cause no issue on their own.

Top-level subobject

top-level.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    data "A.B" hex"11223344556677"
}
solc --strict-assembly top-level.yul --asm
Error: Unknown data object "A.B".
 --> top-level.yul:3:28:
  |
3 |         sstore(0, datasize("A.B"))
  |                            ^^^^^

The error is coming from Yul analysis, which means that it treats it as a path and expects a nested subobject.

Nested subobject

nested.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    object "A" {
        code {}

        data "B" hex"112233"
    }
}
solc --strict-assembly nested.yul --asm
======= nested.yul (EVM) =======
Uncaught exception:
/solidity/libyul/Object.cpp(155): Throw in function std::vector<long unsigned int> solidity::yul::Object::pathToSubObject(solidity::yul::YulString) const
Dynamic exception type: boost::wrapexcept<solidity::yul::YulAssertion>
std::exception::what: Assembly object <A.B> not found or does not contain code.
[solidity::util::tag_comment*] = Assembly object <A.B> not found or does not contain code.

This one probably passes Yul analysis but then fails later at a place that interprets the path differently.

Ambiguous subobject

ambiguous.yul:

object "X" {
    code {
        sstore(0, datasize("A.B"))
    }

    data "A.B" hex"11223344556677"

    object "A" {
        code {}

        data "B" hex"112233"
    }
}
solc --strict-assembly ambiguous.yul --asm --debug-info none
======= ambiguous.yul (EVM) =======

Text representation:
  0x07
  0x00
  sstore
  stop
stop
data_c304e60a0d3af7489b64bc5b186db2aed0df052c89a05935e0731015cd85d049 11223344556677

sub_0: assembly {
      stop
    stop
    data_50fceab2fe7ed15023d21b343e098d8a822f44ed61ba7e988e708db9c68c2535 112233
}

Note the 0x07 - when assembling we're apparently choosing the top-level data, again different from analysis.

cameel commented 1 month ago

Decision from the call today: we're going to disallow dots in names and we don't consider it breaking. Such names don't seem to work properly outside of weird corner cases anyway and any code broken by it can be trivially converted into working code by just removing the dots. It also only affects Yul - the Solidity compiler never generates object names with dots by itself.

salvadormartin3z commented 1 month ago

@cameel Hello, I hope you're doing well. I noticed that this issue has a "low effort" label, and I would be interested in contributing. Is there a possibility that I could help with something?

I'm just starting to study Solidity, and I created a repo to track my progress: https://github.com/salvadormartin3z/Learn-Solidity any recommendations are welcome.

I hope we can connect.