FortKnoxster - Audit

In the contract `DieFiForwarder` the role `DEFAULT_ADMIN_ROLE` has authority over the functions shown in the diagram below and the following functions: * `grantRole()` * `revokeRole()` Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow the hacker to take advantage of this authority and: * Pause or unpause the function `execute()` from executing meta transactions by the `RELAY_ROLE` address, with functions `pause()` and `unpause()`. * Grant or revoke the `RELAY_ROLE` to/from any address with functions `grantRole()` and `revokeRole()`. A user with the `DEFAULT_ADMIN_ROLE` also has the ability to grant/revoke this role to/from any address, and grant/revoke the `RELAY_ROLE` that is described below.  --- In the contract `DieFiForwarder` the role `RELAY_ROLE` has authority over the functions shown in the diagram below. Any compromise to the `RELAY_ROLE` account may allow the hacker to take advantage of this authority and: * Verify a signature against the given `ForwardRequest` variables and execute the meta transaction in place of the signature signer.  --- In the contract `DieFiPolicy` the role `DEFAULT_ADMIN_ROLE` has authority over the functions shown in the diagram below and the following functions: * `grantRole()` * `revokeRole()` Any compromise to the `DEFAULT_ADMIN_ROLE` account may allow the hacker to take advantage of this authority and: * Grant or revoke the `DEFAULT_ADMIN_ROLE` to or from any address. The `DEFAULT_ADMIN_ROLE` has the ability to call the function described below. * Change the `subscriptionManager` address to any address with function `setSubscriptionManager()`. The `subscriptionManager` address is used throughout the contract to call external `policy` functions. 

A reentrancy attack can occur when the contract creates a function that makes an external call to another untrusted contract before resolving any effects. If the attacker can control the untrusted contract, they can make a recursive call back to the original function, repeating interactions that would have otherwise not run after the external call resolved the effects. If a recursive call is preformed on either function `execute()` or `createPolicy()`, the ordering of events emitted will be inaccurate compared to the actual execution timeline of the external calls. ### External call ```solidity=62 (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}( abi.encodePacked(req.data, req.from) ); ``` ### Events emitted after the call ```solidity=78 emit MetaTransactionExecuted(req.from, req.to, req.data); ``` --- ### External call ```solidity=66 ISubscriptionManager(subscriptionManager).createPolicy{value: msg.value }( _policyId, _policyOwner, _size, _startTimestamp, _endTimestamp ); ``` ### Events emitted after the call ```solidity=74 emit DieFiPolicyCreated( _policyId, _policyOwner, _size, _startTimestamp, _endTimestamp, msg.value ); ```

The functions which are never called internally within the contract should have external visibility for gas optimization.