BBB-01 | Lack Of Input Validation #3171

[Brando753]

Added input validation on BridgeBank _initialize function.

[banshee]

Should we also add:

diff --git a/smart-contracts/contracts/BridgeBank/BridgeBank.sol b/smart-contracts/contracts/BridgeBank/BridgeBank.sol
index e9757073a..f48bdbdbc 100644
--- a/smart-contracts/contracts/BridgeBank/BridgeBank.sol
+++ b/smart-contracts/contracts/BridgeBank/BridgeBank.sol
@@ -147,7 +147,9 @@ contract BridgeBank is BankStorage, CosmosBank, EthereumWhiteList, CosmosWhiteLi
     require(_operator != address(0), "invalid _operator address");
     require(_cosmosBridgeAddress != address(0), "invalid _cosmosBridgeAddress address");
     require(_owner != address(0), "invalid _owner address");
+    require(_pauser != address(0), "invalid _pauser address");
     require(_networkDescriptor >= 0 && _networkDescriptor <= 9999, "invalid _networkDescriptor");
+    require(_rowanTokenAddress != address(0), "invalid _rowanTokenAddress address");
 
     Pausable._pausableInitialize(_pauser);
 

[banshee]

And should we shorten the error strings?

diff --git a/smart-contracts/contracts/BridgeBank/BridgeBank.sol b/smart-contracts/contracts/BridgeBank/BridgeBank.sol
index e9757073a..ca3528a8b 100644
--- a/smart-contracts/contracts/BridgeBank/BridgeBank.sol
+++ b/smart-contracts/contracts/BridgeBank/BridgeBank.sol
@@ -144,10 +144,12 @@ contract BridgeBank is BankStorage, CosmosBank, EthereumWhiteList, CosmosWhiteLi
     address _rowanTokenAddress
   ) private {
     
-    require(_operator != address(0), "invalid _operator address");
-    require(_cosmosBridgeAddress != address(0), "invalid _cosmosBridgeAddress address");
-    require(_owner != address(0), "invalid _owner address");
-    require(_networkDescriptor >= 0 && _networkDescriptor <= 9999, "invalid _networkDescriptor");
+    require(_operator != address(0), "_operator");
+    require(_cosmosBridgeAddress != address(0), "_cosmosBridgeAddress");
+    require(_owner != address(0), "_owner");
+    require(_pauser != address(0), "_pauser");
+    require(_networkDescriptor >= 0 && _networkDescriptor <= 9999, "_networkDescriptor");
+    require(_rowanTokenAddress != address(0), "_rowanTokenAddress");
 
     Pausable._pausableInitialize(_pauser);
 

[smartyalgo]

network descriptor of 0 shouldn't be allowed. It is the default when not specified.

[smartyalgo]

Not required for this ticket, we should add the same checks for

[Brando753]

Null address is a valid RowanAddress as this value would only be set on Ethereum which is the network which held eRowan.