fix(nexus)!: prevent messages from wasm to use nexus registered source chain #2180
Conversation
milapsheth
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2180 +/- ##
=======================================
Coverage 49.48% 49.48%
=======================================
Files 272 272
Lines 16151 16153 +2
=======================================
+ Hits 7992 7993 +1
- Misses 7547 7548 +1
Partials 612 612 ☔ View full report in Codecov by Sentry. |
| // Prevent wasm gateway from spoofing a chain name registered in nexus as the source chain | ||
| if _, ok := m.GetChain(ctx, msg.SourceChain); ok { | ||
| return fmt.Errorf("wasm source chain %s should not be a registered chain in nexus", msg.SourceChain) | ||
| } |
There was a problem hiding this comment.
While this is a good sanity check, I am not sure this is the best place for the check. It would be better to somehow handle this at chain registration. Also, out of curiosity, what would be the harm in allowing the same chain name to be registered in both places?
There was a problem hiding this comment.
It would be better to somehow handle this at chain registration.
Checking at chain registration doesn't prevent the wasm layer to spoof it dynamically when routing a GMP call to core. This ensures that it's never possible on the core side. We should still add it at the chain registration in amplifier though for sanity.
Also, out of curiosity, what would be the harm in allowing the same chain name to be registered in both places?
The chain name is part of the root of trust for a GMP call. Apps whitelisting by chain (e.g. ITS) are relying on a certain standard of security for the chain connection. The connection type can be completely different between core and amplifier for the same chain, so they should be distinguished in GMP by different identifiers. And in the event of a compromise on the wasm layer, we don't want to allow it to spoof consensus chains. Apps that only trusted consensus chains could now get a malicious message from wasm acting as one.
|
We decided to check this during chain registration instead |
AXE-4844
Description
Todos
Steps to Test
Expected Behaviour
Other Notes