PeriodicPriceStrategy - trustedForwarder can impersonate any msg.sender
The `trustedForwarder` undermines the trust assumptions in the system. For example, one would assume that the access control modifier `onlyPrizePool` would only allow the configured `PrizePool` to call certain methods. However, in reality, the `trustedForwarder` can assume this position as well. The same is true for the `onlyOwnerOrListener` modifier. One would assume `msg.sender` must either be `periodicPrizeStrategyListener` or `owner` (the initial deployer) while the `trustedForwarder` can assume any of the administrative roles.\\nThe centralization of power to allow one account to impersonate other components and roles (owner, `listener`, prizePool) in the system is a concern by itself and may give users pause when deciding whether to trust the contract system. The fact that the `trustedForwarder` can spoof events for any `msg.sender` may also make it hard to keep an accurate log trail of events in case of a security incident.\\nNote: The same functionality seems to be used in `ControlledToken` and other contracts which allows the `trustedForwarder` to assume any tokenholder in `ERC20UpgradeSafe`. There is practically no guarantee to `ControlledToken` holders.\\nNote: The trustedForwarder/msgSender() pattern is used in multiple contracts, many of which are not in the scope of this assessment.\\naccess control modifiers that can be impersonated\\n```\\nmodifier onlyPrizePool() {\\n  require(\\_msgSender() == address(prizePool), "PeriodicPrizeStrategy/only-prize-pool");\\n  \\_;\\n}\\n```\\n\\n```\\nmodifier onlyOwnerOrListener() {\\n  require(\\_msgSender() == owner() || \\_msgSender() == address(periodicPrizeStrategyListener), "PeriodicPrizeStrategy/only-owner-or-listener");\\n  \\_;\\n}\\n```\\n\\nevent `msg.sender` that can be spoofed because the actual `msg.sender` can be `trustedForwarder`\\n```\\nemit PrizePoolOpened(\\_msgSender(), prizePeriodStartedAt);\\n```\\n\\n```\\nemit PrizePoolAwardStarted(\\_msgSender(), address(prizePool), requestId, lockBlock);\\n```\\n\\n```\\nemit PrizePoolAwarded(\\_msgSender(), randomNumber);\\nemit PrizePoolOpened(\\_msgSender(), prizePeriodStartedAt);\\n```\\n\\n`_msgSender()` implementation allows the `trustedForwarder` to impersonate any `msg.sender` address\\n```\\n/// @dev Provides information about the current execution context for GSN Meta-Txs.\\n/// @return The payable address of the message sender\\nfunction \\_msgSender()\\n  internal\\n  override(BaseRelayRecipient, ContextUpgradeSafe)\\n  virtual\\n  view\\n  returns (address payable)\\n{\\n  return BaseRelayRecipient.\\_msgSender();\\n}\\n```\\n
Remove the `trustedForwarder` or restrict the type of actions the forwarder can perform and don't allow it to impersonate other components in the system. Make sure users understand the trust assumptions and who has what powers in the system. Make sure to keep an accurate log trail of who performed which action on whom's behalf.
```\\nmodifier onlyPrizePool() {\\n  require(\\_msgSender() == address(prizePool), "PeriodicPrizeStrategy/only-prize-pool");\\n  \\_;\\n}\\n```\\n