Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add FirewalledRule #200

Merged
merged 5 commits into from
May 2, 2019
Merged

Add FirewalledRule #200

merged 5 commits into from
May 2, 2019

Conversation

schemar
Copy link
Contributor

@schemar schemar commented Apr 24, 2019

I added a FirewalledRule which can be extended in order to restrict
relaying of meta-transactions to organization workers.
The rule that is to be restricted must extend the FirewalledRule and add
the modifier firewalled to all relevant functions.

For testing I created two test contract files. One that uses the
modifier, and one that calls the one that uses the modifier. Required in
order to simulate rules calling on each other and checking for the
relayer.

Added a step to manually install npx inside docker until the docker
image is fixed. The problem is reported and confirmed at
trailofbits/eth-security-toolbox#8

Fixes #199

I added a FirewalledRule which can be extended in order to restrict
relaying of meta-transactions to organization workers.
The rule that is to be restricted must extend the FirewalledRule and add
the modifier `firewalled` to all relevant functions.

For testing I created two test contract files. One that uses the
modifier, and one that calls the one that uses the modifier. Required in
order to simulate rules calling on each other and checking for the
relayer.

Added a step to manually install npx inside docker until the docker
image is fixed. The problem is reported and confirmed at
trailofbits/eth-security-toolbox#8

Fixes OpenST#199
@schemar
Copy link
Contributor Author

schemar commented Apr 25, 2019

Crytic fixed the docker image, I should test and remove the extra line before merging!

@schemar
Copy link
Contributor Author

schemar commented Apr 26, 2019

When they fixed the previously reported bug, it seems they broke something else. I made a new report: trailofbits/eth-security-toolbox#9

Waiting for an update, as this cannot be fixed with a workaround.

@pgev pgev self-assigned this Apr 30, 2019
pgev
pgev previously approved these changes Apr 30, 2019
Copy link
Collaborator

@pgev pgev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Once Travis fixed we could merge it.

@pgev pgev removed their assignment May 2, 2019
@schemar
Copy link
Contributor Author

schemar commented May 2, 2019

Travis CI cannot cache docker images (https://docs.travis-ci.com/user/caching/#things-not-to-cache):

Docker images are not cached, because we provision a brand new virtual machine for every build.

I cannot get the security toolbox to work.
@schemar
Copy link
Contributor Author

schemar commented May 2, 2019

@pgev can you please have a look? I removed slither from make:all. I cannot get slither work with the eth-security-toolbox.

Copy link
Collaborator

@pgev pgev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. I've also subscribed to the ticket you've created (Slither fails to execute in latest docker tag). Once it's fixed we should create follow ticket to bring slither back.

@pgev pgev merged commit 42bc1d4 into OpenST:develop May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repository should provide abstract firewall contract
2 participants