Load evmone from a trusted absolute path and verify its SHA-256#3624
Conversation
InitEvmoneVM located the evmone shared library via runtime.Caller(0), which returns the compile-time source path. That path does not exist on a deployed node, so evmc.Load always failed there. Resolving the library by bare name (as proposed in #3613) would instead hand the lookup to the dynamic linker's search path (LD_LIBRARY_PATH, ld.so.cache, default dirs). For a consensus-critical process that is an untrusted-search-path risk: any library found earlier in that order is loaded and executed, and the load is decoupled from the SHA-256 the generator pins. Resolve the library to a trusted absolute path instead ($SEI_EVMONE_LIB_DIR, then /usr/lib, then the source tree for local dev/tests) and verify its SHA-256 against a per-platform pinned digest before handing it to evmc.Load; an absolute path makes dlopen open the file directly and skip the search path entirely. Install the library into /usr/lib in the release image so production resolves it, and add a test asserting the checked-in library matches the pinned digest.
PR SummaryHigh Risk Overview Release Docker images copy the platform Reviewed by Cursor Bugbot for commit fc13ac4. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3624 +/- ##
==========================================
- Coverage 58.86% 57.97% -0.90%
==========================================
Files 2225 2151 -74
Lines 183443 174920 -8523
==========================================
- Hits 107986 101406 -6580
+ Misses 65765 64523 -1242
+ Partials 9692 8991 -701
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc13ac42dd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if dir := os.Getenv(libDirEnv); dir != "" { | ||
| dirs = append(dirs, dir) |
There was a problem hiding this comment.
Reject untrusted override directories
When SEI_EVMONE_LIB_DIR is set, this accepts the value without checking that it is absolute or non-writable, even though the loader's security property depends on using a trusted absolute path. In deployments that set this override to a relative path or to a directory writable by another local user, the digest is checked on one open and then evmc.Load reopens the path afterward, making the load cwd-dependent and leaving a TOCTOU window for replacing the file. Please reject non-absolute override paths and validate the directory ownership/permissions before using it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Given this vector is only exploitable on a local node, and the possibility of evmone being removed completely from the codebase I choose to not reflect on this feedback. The feedback itself is sound.
Thank you robot overlords.
InitEvmoneVM located the evmone shared library via runtime.Caller(0), which returns the compile-time source path. That path does not exist on a deployed node, so evmc.Load always failed there.
Resolving the library by bare name (as proposed in #3613) would instead hand the lookup to the dynamic linker's search path (LD_LIBRARY_PATH, ld.so.cache, default dirs). For a consensus-critical process that is an untrusted-search-path risk: any library found earlier in that order is loaded and executed, and the load is decoupled from the SHA-256 the generator pins.
Resolve the library to a trusted absolute path instead ($SEI_EVMONE_LIB_DIR, then /usr/lib, then the source tree for local dev/tests) and verify its SHA-256 against a per-platform pinned digest before handing it to evmc.Load; an absolute path makes dlopen open the file directly and skip the search path entirely. Install the library into /usr/lib in the release image so production resolves it, and add a test asserting the checked-in library matches the pinned digest.