Git Product home page Git Product logo

Comments (7)

f4z3r avatar f4z3r commented on June 9, 2024

I believe the sloc version is meant to be installed based on the input solidity files. Therefore hardcoding a version into Dockerfile will not fix the issue if a contract with a non compatible solidity pragma is given as input.

Right now the automatic install of sloc is handled by scripts/install_sloc.py which installs the latest version (currently 0.4.25) which is too high for the example contract (pragma solidity 0.4.24;). I would rather propose to change the install script to add:

if not os.path.exists(binary):
    install_solc(f'v{solc_version}', platform='linux')

after the binary path is set. This should then install the correct sloc version based on the contracts provided if no install already exists.

Of course, this does not solve the Killed message.

from securify.

mds1 avatar mds1 commented on June 9, 2024

Good point about scripts/install_solc.py. I had overlooked that, and just assumed it was using the hardcoded solc version installed in lines 34-36 of the original Dockerfile, shown here:

securify/Dockerfile

Lines 34 to 36 in 13d4784

# install solc
RUN wget https://github.com/ethereum/solidity/releases/download/v0.4.24/solc-static-linux -O /usr/local/bin/solc &&\
chmod u+x /usr/local/bin/solc

Now, though, why would we need the above lines in Dockerfile if solc is later installed using scripts/install_solc.py? I think those lines can be removed.

If I understood your proposal correctly, I believe you are suggesting to add those lines into pysolc.py, right after the binary path is determined on line 116, i.e. like this:

def compile_solfiles(files, proj_dir, solc_version=None, output_values=OUTPUT_VALUES):
    
    # original function code was here...

    if solc_version is None:
        solc_version = min(map(parse_version, files),
                           key=lambda x: _version_to_tuple(x))

    binary = os.path.join(Path.home(), f'.py-solc/solc-v{solc_version}/bin/solc')

    # proposing to add these two lines
    if not os.path.exists(binary):
        install_solc(f'v{solc_version}', platform='linux')

    # ...continue original function code

This seems like a good solution, since it allows the image to always use the proper solc version without rebuilding the image. It also means you can remove scripts/install_solc.py and the corresponding line in Dockerfile, since solc is now installed on each run.

This combination of changes (removing those Dockerfile lines and installing the proper solc version with each run) seem to work well in my brief testing.

A downside here is that by removing the above Dockerfile lines the image cannot find solc simply by running solc --version

from securify.

f4z3r avatar f4z3r commented on June 9, 2024

Yes, that is where I would place the binary existence check.

I agree that the image not finding sloc simply by running sloc --version is a disadvantage, but considering the sloc executable (and version) provided by this is not the one used to compile the source code, I would actually say it is better to not have at all (it can create confusion). As to why this install is explicit in the Dockerfile in the first place, I have no idea.

Regarding removing the scripts/install_sloc.py altogether, I would argue that installing the latest version of sloc during the building of the docker image is not really problematic and caches at least one version of sloc in the image. This would allow faster analysis on most contracts with non-strict pragmas as the latest version of solc can be used. If this is removed, the existence check of the binary

if not os.path.join(binary):

is useless as no version of sloc will ever be cached between docker container runs. Therefore you would indeed need a fresh install of sloc on each run, which I believe not to be ideal.

from securify.

f4z3r avatar f4z3r commented on June 9, 2024

I don't know much about Dockerfiles or docker in general, but maybe a smart solution would be to let the user decide on which additional versions of sloc he wants to have preinstalled in the docker image, hence not requiring an explicit install each run. I don't know if this is possible by just passing arguments to the Dockerfile? I think that would be a good (at least temporary) solution because the user should know which solidity versions he is most likely to use.

from securify.

mds1 avatar mds1 commented on June 9, 2024

I'm no Docker expert either, but my understanding of the typical Docker use case makes caching a user-chosen solc between runs not very feasible.

To elaborate, a typical use case would be:

# Step 1: Pull an already built Securify image from https://hub.docker.com/ 
#   This image will either have a chosen version of `solc` pre-installed, or no
#   version at all, depending on the decision made 
docker pull eth-sri/securify

# Step 2: Run Securify on your contracts
docker run -v $(pwd)/folder_with_solidity_files:/project eth-sri/securify

Note on step 1: Currently there is no image for this project on Docker Hub, but it looks like it'll happen eventually in #3.

The command in step 2 simply runs docker_run_securify within the container. This would fetch the appropriate version of solc, if necessary, and then exits the container. A newly installed version of solc does not persist between runs of this command, as containers do not have persistent storage by default. On a related note, with additional changes, you could allow the user to pass in a solc version using an environment variable like this:

docker run -v $(pwd)/folder_with_solidity_files:/project -e SOLC_VERSION=0.4.23 eth-sri/securify

Then you could check for the existence of the SOLC_VERSION environment variable, and if it exists, download that version. But once the container exits, re-running the above command generates a new container and solc must again be installed, even if using the same non-default version as the last call.

Personally, I think it's ok even if no solc versions are installed by default, since the install is pretty quick. Including the latest solc version with the image certainly can't hurt though.

To ensure users know what solc version is being used, you could either (1) force users to specify versions with the environment variable, and/or (2) include the solc version used in the JSON output

from securify.

f4z3r avatar f4z3r commented on June 9, 2024

Oh yes never mind I did not consider that the image would be pulled from Docker Hub instead of installed by the user directly. So I guess the pre-install on the image is not really necessary / very useful.

from securify.

hiqua avatar hiqua commented on June 9, 2024

The problem was that we only installed the latest version of solc, so the version will change whenever there is a new release. We now install all versions, it means that the docker image is slower to build (which is why I wanted to avoid this), but it will do for now.

from securify.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.