Zoksh Pay security record

In contract GatewayRoute the merchants are able to take their share of fare by calling the settle() function. The other function refund() is used for refunding the users if the wrong purchase was made or for some reason where the user/customer didn’t like the product. But the merchant can pass his other address as the recipient to take all the money which allows the merchant to bypass Zoksh’s fees.

In contract GatewayRoute the merchants are able to take their share of fare by calling the settle() function. The other function refund() is used for refunding the users if the wrong purchase was made or for some reason where the user/customer didn’t like the product. But the merchant can pass his other address as the recipient to take all the money which allows the merchant to bypass Zoksh’s fees.

The GatewayFactory contract has the variable setFee initialized to 100. From the calculations in _settle() on L112 this would cause 100% of the amount to be deducted as fees, this way preventing the merchant from withdrawing any amounts and transferring all tokens to the gatewayAddr.Currently in the setFee() function the admin can pass in any value to adjust the fee and the merchant uses confirmFee() to accept the adjusted fee variable. If the fee set is much higher than expected, it can limit user interaction with the protocol.

In GatewayRoute, function settle() and refund() send is used for transferring the ether. Future changes or forks in Ethereum can result in higher gas fees than send provides. The .send() creates a hard dependency on 2300 gas units being appropriate now but not in the future. This may also affect functionality when a multi-sig wallet is used for such interactions.

For ease of searching and the use of monitoring tools the following events in GatewayRoute can be indexed, PaymentSettled(), PaymentRefunded(), ManagerUpdated(). In the emission of events as well, there is no need to cast an address to payable.In GatewayFactory, the RouteCreated event has a 4th event topic without a name.

For ease of searching and the use of monitoring tools the following events in GatewayRoute can be indexed, PaymentSettled(), PaymentRefunded(), ManagerUpdated(). In the emission of events as well, there is no need to cast an address to payable.In GatewayFactory, the RouteCreated event has a 4th event topic without a name.

Functions with an external visibility modifier are not called directly in the contract and are cheaper than public functions are. For functions that are not to be called internally, it is best practice to make them external for gas savings.In GatewayRoute, getBlocked(), getManager(), getGatewayAddr() and getAdminContract() can be declared external. In GatewayAddress, getGatewayAddress() and getAdminContract() can be declared external.

Functions with an external visibility modifier are not called directly in the contract and are cheaper than public functions are. For functions that are not to be called internally, it is best practice to make them external for gas savings.In GatewayRoute, getBlocked(), getManager(), getGatewayAddr() and getAdminContract() can be declared external. In GatewayAddress, getGatewayAddress() and getAdminContract() can be declared external.

The naming convention used in the codebase is not clear and reduces the readability of the entire codebase. The terms manager and merchant are used concurrently but they could refer to different entities.In GatewayFactory, the onlyManager() modifier checks that the caller is the same as the merchantAddr address provided in the Administrable (GatewayAccess) contract. Since the check is for the merchant and not the manager, it can be changed to onlyMerchant(). In GatewayRoute, the onlyMerchant() modifier checks that the caller is the same as the managerAddr address provided in the creation of the GatewayRoute contract. Since the check is for the manager and not the manager, it can be changed to onlyManager().

From GatewayFactory, the creation of a route can happen with createRoute() or deployRoute(). When routes are created using deployRoute() the merchant address and manager address are the same (msg.sender), but with createRoute() the manager can be any value passed in. The issue here is that the merchant no longer has the privileges to call onlyMerchant() functions if a separate manager is set when calling createRoute().