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

security: automatic remappings can be dangerous #9146

Open
2 tasks done
Tracked by #9157
QGarchery opened this issue Oct 19, 2024 · 1 comment · Fixed by #9246
Open
2 tasks done
Tracked by #9157

security: automatic remappings can be dangerous #9146

QGarchery opened this issue Oct 19, 2024 · 1 comment · Fixed by #9246
Assignees
Labels
A-remappings Area: remappings P-normal Priority: normal T-bug Type: bug
Milestone

Comments

@QGarchery
Copy link

QGarchery commented Oct 19, 2024

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (a8c3e9c 2024-10-19T00:21:12.472031180Z)

What command(s) is the bug in?

forge build

Operating System

Linux

Describe the bug

Forge infers remappings by taking into account remappings of sub-projects. When there are conflicting remappings, it seems like the longest / most specified has the priority. This can be dangerous, as adding sub-projects can now completely change the code that is executed, even if the remappings of the root project are not changed.

Reproduced in this test. Notice that removing the lib/interesting-project makes the test pass again. Also notice how interesting-project can be very deep in the sub-projects, so it can be difficult to detect that the executed bytecode has changed.

Proposed solution: make remappings of the root project have priority over remappings of sub-projects.

@QGarchery QGarchery added T-bug Type: bug T-needs-triage Type: this issue needs to be labelled labels Oct 19, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Oct 19, 2024
@zerosnacks zerosnacks changed the title Automatic remappings can be dangerous security: automatic remappings can be dangerous Oct 21, 2024
@zerosnacks zerosnacks added P-normal Priority: normal T-to-investigate Type: to investigate A-remappings Area: remappings and removed T-needs-triage Type: this issue needs to be labelled labels Oct 21, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Oct 22, 2024
@grandizzy grandizzy self-assigned this Nov 4, 2024
@grandizzy grandizzy moved this from Todo to Ready For Review in Foundry Nov 4, 2024
@github-project-automation github-project-automation bot moved this from Ready For Review to Done in Foundry Nov 5, 2024
@grandizzy
Copy link
Collaborator

reopen per #9274

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-remappings Area: remappings P-normal Priority: normal T-bug Type: bug
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants
@QGarchery @grandizzy @zerosnacks and others