harmony-one / horizon

Horizon - a trustless harmony to ethereum bridge
MIT License
36 stars 29 forks source link

Accessibility modifiers inside token registry #33

Closed brucdarc closed 2 years ago

brucdarc commented 2 years ago

Both the functions onTokenMapAckEvent and onTokenMapReqEvent inside tokenRegistry.sol are intended to be executed when the correct event is verified in a block through the execute function of tokenLocker.sol (which inherits tokenRegistry).

Currently these functions are public, meaning anyone can call them. This means an attacker could add invalid entries to the internal mappings through these functions. From what I can tell the severity of this is limited to causing denial of service to the bridge, due to proper require statements.

Since it seems that these functions are meant to be triggered through execute, parsing an event from another chain, I would recommend these functions be internal. In this case the only public function should be issueTokenMapReq, which starts the req and ack communication between chains.

Also the issueTokenMapReq function was declared to return an address, but none was actually being returned so I updated its function signature.