Open c4-bot-4 opened 9 months ago
QA might be more appropriate
DadeKuma marked the issue as insufficient quality report
going to leave open for sponsor comment before downgrading.
lumtis (sponsor) confirmed
Confirm validity. Need to be updated.
while this is very well documented issue, I don't see how it is more than QA at this point.
It is essentially functionality not meeting the specification with little impact beyond that as both of these calls are privileged already.
0xean changed the severity to QA (Quality Assurance)
0xean marked the issue as grade-a
Lines of code
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/observer/keeper/msg_server_update_crosschain_flags.go#L12-L58
Vulnerability details
Impact
ZetaChain implements a critical separation of privileges by using two types of Admin Groups. According to "Important Areas" documentation and the implementation itself, certain actions can only be executed by one group or the other.
Group1
is assigned for emergency actions that need rapidity over security, and is represented by an address that requires a single signature from its multisig scheme.Group2
is assigned for operational actions influencing the logic of the blockchain that need security over rapidity, and is represented by an address that requires several signatures from its multisig scheme.The issue is that
Group1
is allowed to enable/disable the crosschain flagflags.BlockHeaderVerificationFlags
. This not only differs from the "Important Areas" documentation, but is also a violation of the designed separation of privileges. More specifically, enabling theflags.BlockHeaderVerificationFlags
cannot be justified as an emergency action and activates (instead of deactivating in case of emergency) critical operations in the blockchain. This renders a critical flag for the blockchain operation mistakenly open to only one address in theGroup1
multisig.Proof of Concept
This is the relevant (fixed) part of the "Permissions Table" in the "Important Areas" documentation:
Admins in the
Group1
should only be able to disable outbounds/inbounds for the case of emergencies, while all other actions should be reserved toGroup2
. But this is not what the code in theUpdateCrosschainFlags
message function enforces:https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/observer/keeper/msg_server_update_crosschain_flags.go#L12-L58
The
flags.BlockHeaderVerificationFlags
can be switched by bothGroup1
andGroup2
.These flags are used in:
msg_server_add_block_header.go
, to enable or disable the addition of new block headers for each chain for voting;verify_proof.go
, to enable or disable the proof verification. Effectively, this enables or disables users from submitting transactions proofs for each chain to be added to the trackers via msg_server_add_to_intx_tracker.go and msg_server_add_to_outtx_tracker.go.A simple unit test can be included in contest repository's file
repos/node/x/observer/keeper/msg_server_update_crosschain_flags_test.go
to test for the authorization flaw:Then run:
Tools Used
Manual: code editor.
Recommended Mitigation Steps
One potential solution that seems to adhere to the intended security model is to emulate what is done to the other critical flags and allow
Group1
to only disable theflags.BlockHeaderVerificationFlags
, for the cases of emergency situations. Or, if intended, just remove completely theGroup1
permission to setflags.BlockHeaderVerificationFlags
.Assessed type
Access Control