Skip to content

Conversation

muhammad-hassnain
Copy link

Currently cbindgen looks up the CARGO environment variable and falls back to "cargo" if unset. This means any process that can influence the environment (e.g. by setting CARGO=/tmp/evil) could cause cbindgen to run an arbitrary binary with the privileges of the invoking process.

This patch changes the callsites in cargo_expand.rs and cargo_metadata.rs to invoke "cargo" directly instead of honoring $CARGO.

The call sites are linked below:

  1. src/bindgen/cargo/cargo_expand.rs:72 : Link
  2. src/bindgen/cargo/cargo_metadata.rs:234 : Link

Benefits:

  • Principle of least privilege: reduces attack surface in environments where $CARGO may be attacker-controlled (e.g. services, CI).
  • Behavior is more predictable and consistent across setups.

Potential trade-off:

  • Builds that intentionally override CARGO to point to a non-default binary will no longer be respected. If that compatibility is important, an alternative would be to validate $CARGO rather than ignore it completely.

Copy link
Collaborator

@emilio emilio left a comment

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 makes sense. If an attacker can modify the environment it can also modify PATH to make cargo be whatever it wants, or LD_LIBRARY_PATH / LD_PRELOAD to run more arbitrary code. Am I missing something?

@emilio
Copy link
Collaborator

emilio commented Sep 6, 2025

Similarly real cargo will honor things like RUSTC and so on I believe...

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.

2 participants