code-423n4 / 2022-03-rolla-findings

1 stars 1 forks source link

assets can be added twice #32

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/AssetsRegistry.sol#L55

Vulnerability details

the check that addAsset makes in order to check that an asset is not already added is

bytes(assetProperties[_underlying].symbol).length==0

however, when adding a new asset the symbol can be set to an empty string. therefore an asset can be added multiple times.

Proof of Concept

Alice adds an asset and sets symbol="". then she accidently adds it again and it works. now registeredAssets contains this asset twice.

quantizations commented 2 years ago

Disputed because unreasonable to add asset with empty symbol.

alcueca commented 2 years ago

Adding an asset is a governance function, and the damage from a duplicated asset is not detailed by the warden. Downgraded to QA issue.

Adding an asset with an empty symbol is unusual, but not unreasonable.

As a note to the sponsor, you have free space in the relevant struct, and adding a bool set field would make the code clearer and more robust at no cost.

alcueca commented 2 years ago

Score as QA report: 33 (including points from #33 and #36)

JeeberC4 commented 2 years ago

Including with QA Report #36