code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

QA Report #59

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

create can be executed before setMarketPlace by mistake, creating a ZcToken with wrong marketPlace

Target codebase

Creator.sol#L54-L61

Vulnerability details

Impact

Before deploy the Creator contract the owner can call the create function before setMarketPlace that don't check the marketPlace and by mistake can create a invalid ZcToken

Proof of ConceptThe function updates the fenominators[x] instead of feenominators[i[x]]

Recommended Mitigation Steps

Should remove the function setMarketPlace and add marketPlace to constructor.

- address public marketPlace;
+ address public immutable marketPlace;

constructor(address m) {
  admin = msg.sender;
+ marketPlace = m;
}

- function setMarketPlace(address m) external authorized(admin) returns (bool) {
-  if (marketPlace != address(0)) {
-    revert Exception(33, 0, 0, marketPlace, address(0)); 
-  }
-  marketPlace = m;
-  return true;
- }

Natspec issue

Target codebase

VaultTracker.sol#L31

Swivel.sol#L615

Vulnerability details

Impact

Natspec is incorrect or missing

Recommended Mitigation Steps

Should fix the following natspec:

In Swivel.sol:
- /// @notice p Protocol Enum value associated with this market pair
+ /// @param p Protocol Enum value associated with this market pair

In VaultTracker

+ /// @param p Protocol Enum value associated with this market pair
  /// @param m Maturity timestamp associated with this vault
  /// @param c Compounding Token address associated with this vault
  /// @param s Address of the deployed swivel contract
  constructor(uint8 p, uint256 m, address c, address s) {
robrobbins commented 2 years ago

creator has to go out first, as marketplace expects it's address at construction, it is thus the responsibily of the deploying admin to assure that address is set.