Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #118

Open
code423n4 opened this issue Feb 23, 2022 · 2 comments
Open

QA Report #118

code423n4 opened this issue Feb 23, 2022 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@code423n4
Copy link
Contributor

LOW :
1.
Title : Missing limit on how many AMMs can be added

Impact :
The governance can add an amm, by calling whitelistAmm function, however there is no limit on how many amm that the contract can be held, if the governance keep adding amm, then the clearing house will brick with out of gas, since all other user is interacting with the clearing house and the main functionality of this contract is updatePoition and this function is being called by removeLiquidity, addLiquidity, openPosition, closePosition function

POC :
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L341

Title : Missing check on duplicate amm

Impact :
There is missing check on axisting amm, and amm that will going to be added from whitelistAmm function, since there is no check whether the same amm is already being added or not, a multiple amm might be added without error.

POC :
https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/ClearingHouse.sol#L341

Title : Grieve in processWtihdrawals

Impact : an attacker could grieve other user by burning many small amount VUSD, to inflate the withdrawals length until 99, and if the victim want to burn their VUSD to USDC, the victim will be placed in the 100, and when the victim want to take the USDC, by calling processWithdrawal, the victim will pay extra fee, that's because the victim must process the withdrawal that the attacker make 99 times, before the victim accept their USDC.

POC :

  1. An attacker mint 100 wei VUSD, with USDC
  2. Burn 1 wei VUSD
  3. Repeat step 2 99 times
  4. The victim use burn function to get USDC back, however the victim must pay extra gas, just to send 1 wei that was belong to the attacker.

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

@code423n4 code423n4 added QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax bug Something isn't working labels Feb 23, 2022
code423n4 added a commit that referenced this issue Feb 23, 2022
@atvanguard
Copy link
Collaborator

Duplicate of #119, #97, #50

@atvanguard atvanguard added the duplicate This issue or pull request already exists label Feb 26, 2022
@moose-code moose-code added the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Mar 6, 2022
@moose-code
Copy link
Collaborator

promote severity for grievance attack

@CloudEllie CloudEllie reopened this Mar 30, 2022
This was referenced Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

4 participants