Repository navigation
fix(wrapper): block forwarding to the management canister - #229
Merged
Merged
Conversation
The bridge only rejected calls whose target was the canister itself, so any caller could set target=aaaaa-aa and have the bridge forward a management-canister call (update_settings, uninstall_code, stop_canister, ...). Such calls execute authorized as the bridge's own principal, which would let an attacker perform lifecycle operations against any canister the bridge controls. Reject the management canister principal explicitly, alongside the existing self-call guard, and cover it with an integration test.
|
✅ No security or compliance issues detected. Reviewed everything up to a317ec6. Security Overview
Detected Code Changes
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the ic-papi-wrapper bridge entrypoints (call0/call_blob) by preventing them from being used as a proxy to the IC management canister (aaaaa-aa), which would otherwise allow forwarded management-canister calls to execute with the bridge canister’s own authority.
Changes:
- Introduces a
BridgeError::ForbiddenTargeterror variant for disallowed bridge targets. - Adds an explicit guard in
bridge_callrejectingPrincipal::management_canister(). - Adds an integration test ensuring bridge calls fail when the target is the management canister.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/wrapper/src/api/call.rs | Adds an explicit management-canister target block inside bridge_call. |
| src/wrapper/src/domain/errors.rs | Adds ForbiddenTarget variant and Display formatting. |
| src/wrapper/tests/it/bridge_tests.rs | Adds integration test for rejecting management-canister targets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
AntonioVentilii
enabled auto-merge (squash)
July 15, 2026 07:35
DenysKarmazynDFINITY
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The bridge's
bridge_callonly rejected calls whosetargetwas the canister itself:It did not block the IC management canister (
aaaaa-aa). Becausecall0/call_blobare public#[update]entrypoints that forward an arbitrarytarget/method/raw args, any caller could settarget = aaaaa-aaand have the bridge forward a management-canister call (update_settings,install_code,uninstall_code,stop_canister, …).On the IC, an inter-canister call is authorized as the calling canister's principal, and management-canister methods authorize on whether the caller is a controller of the
canister_idin the payload. So a forwarded management call executes with the bridge's own authority — if the bridge is (or becomes) a controller of any canister, an attacker could change controllers, uninstall code, or stop that canister.Fix
Reject the management canister principal explicitly, alongside the existing self-call guard, via a new
BridgeError::ForbiddenTargetvariant.Notes on severity
This is primarily a hardening / defense-in-depth fix. The concrete impact is contingent on the bridge controlling some canister; under a default deployment where the bridge controls nothing, the management canister would reject the forwarded call anyway. Blocking
aaaaa-aaat the bridge closes the gap regardless of controller configuration. A longer-term improvement would be moving from a target denylist to an allowlist of permitted(target, method)pairs (theMethodKey/MethodConfigtypes already anticipate this).Testing
cargo build/cargo clippy -p ic-papi-wrapper— cleanbridge_call_fails_if_target_is_management_canisterpasses alongside the existing bridge tests (3 passed).