#36138 [SC-Insight] `Market.update_collateral_asset` should reuse old configuration's `asset_id`

Submitted on Oct 21st 2024 at 15:32:58 UTC by @jasonxiale for IOP | Swaylend

  • Report ID: #36138

  • Report Type: Smart Contract

  • Report severity: Insight

  • Target: https://github.com/Swaylend/swaylend-monorepo/blob/9132747331188b86dd8cbf9a1ca37b811d08dddb/contracts/market/src/main.sw

  • Impacts:

    • Contract fails to deliver promised returns, but doesn't lose value

Description

Brief/Intro

In Market.update_market_configuration, while updating market configuration, the old `base_token` is reused in main.sw#L1307

The same rule should apply to Market.update_collateral_asset, while updating CollateralConfiguration, `CollateralConfiguration.asset_id` should be reused.

Vulnerability Details

As the following code shows, while updating CollateralConfiguration, the function doesn't reuse old `CollateralConfiguration.asset_id` ```Rust 262 #[storage(write)] 263 fn update_collateral_asset(asset_id: AssetId, configuration: CollateralConfiguration) { 264 // Only owner can update collateral asset 265 only_owner(); 266 267 // Check if asset exists 268 require( 269 storage 270 .collateral_configurations 271 .get(asset_id) 272 .try_read() 273 .is_some(), 274 Error::UnknownAsset, 275 ); 276 277 storage 278 .collateral_configurations 279 .insert(asset_id, configuration); 280 281 log(CollateralAssetUpdated { 282 asset_id, 283 configuration, 284 }); 285 } ```

Impact Details

avoid mistake

References

Add any relevant links to documentation or code

Proof of Concept

Proof of Concept

As the following code shows, while updating CollateralConfiguration, the function doesn't reuse old `CollateralConfiguration.asset_id` ```Rust 262 #[storage(write)] 263 fn update_collateral_asset(asset_id: AssetId, configuration: CollateralConfiguration) { 264 // Only owner can update collateral asset 265 only_owner(); 266 267 // Check if asset exists 268 require( 269 storage 270 .collateral_configurations 271 .get(asset_id) 272 .try_read() 273 .is_some(), 274 Error::UnknownAsset, 275 ); 276 277 storage 278 .collateral_configurations 279 .insert(asset_id, configuration); 280 281 log(CollateralAssetUpdated { 282 asset_id, 283 configuration, 284 }); 285 } ```