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

Cosmwasm Cherry security patch (#4954) #5404

Merged
merged 8 commits into from
Jun 6, 2023

Conversation

nicolaslara
Copy link
Contributor

Closes: #4964

What is the purpose of the change

Update wasmvm to deal with https://github.com/CosmWasm/advisories/blob/main/CWAs/CWA-2023-002.md#wasm-module-cache-issue

Testing and Verifying

All tests should pass. Manually verified the version on the compiled binary

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? No
  • Changelog entry added to Unreleased section of CHANGELOG.md? Added

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

* upgrade wasmvm

* get the proper version of wasmvm
@nicolaslara nicolaslara requested a review from a team as a code owner June 2, 2023 10:33
@nicolaslara nicolaslara added the V:state/breaking State machine breaking PR label Jun 2, 2023
@github-actions github-actions bot added the C:app-wiring Changes to the app folder label Jun 5, 2023
@nicolaslara nicolaslara requested a review from p0mvn June 5, 2023 16:02
// v16 upgrade handler
fmt.Println("Running v16 pre-upgrade handler")
// remove the wasm cache for cosmwasm cherry https://github.com/CosmWasm/advisories/blob/main/CWAs/CWA-2023-002.md#wasm-module-cache-issue
err := os.RemoveAll(app.homePath + "/wasm/wasm/cache")
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible someone has that configured under a different path? How should we handle that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is configurable, but I'll double check. If they did and the path is not available as a variable in the code, then I think it's OK to ask for them to delete the directory manually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolaslara
Copy link
Contributor Author

Merging after answering the comment about cache paths

@nicolaslara nicolaslara merged commit 314c771 into main Jun 6, 2023
@nicolaslara nicolaslara deleted the nicolas/cosmwasm-cherry-main branch June 6, 2023 07:11
@github-actions github-actions bot mentioned this pull request Jan 15, 2024
@github-actions github-actions bot mentioned this pull request Mar 1, 2024
@github-actions github-actions bot mentioned this pull request May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:app-wiring Changes to the app folder T:build V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chore: Apply Cosmwasm-cherry on main
2 participants