QA Report

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/VUSD.sol#L11-L11

Vulnerability details

Use of Upgradeable Proxy Contract Structure allows the logic of the contract to be arbitrarily changed.

This allows the proxy admin to perform malicious actions e.g., taking funds from users' wallets up to the allowance limit.

This action can be performed by the malicious/compromised proxy admin without any restriction.

Considering that the purpose of this particular contract is for a token, we believe the smart contract should not be structured as an upgradeable contract.

PoC

Given:

• reserveToken: USDC

1. Alice approve VUSD spending her USDC and mintWithReserve() VUSD;
2. Bob approve VUSD spending his USDC and mintWithReserve() VUSD;
3. A malicious/compromised proxy admin can call upgradeToAndCall() on the proxy contract and set a malicious contract as newImplementation and stolen all the WBTC in Alice's and Bob's wallets;

Severity

A smart contract being structured as an upgradeable contract alone is not usually considered as a high severity risk. But given the severe impact (funds in users' wallets got stolen), with the PoC above, once the proxy admin is compromised, users' funds can be at risk. Therefore, we mark it as a High severity issue.

Recommendation

1. Consider making the contract non-upgradeable.
2. Consider using a non-upgradable contract to hold users' allowances.

[atvanguard]

Duplicate of #40

[JeeberC4]

Since this issue was downgraded to a QA level, and the warden did not submit a separate QA report, we've renamed this one to "QA report" for consistency.
The original title, for the record, was [WP-H1] VUSD.sol Proxy admin of the upgradeable proxy contract can steal users' reserveToken