# Attackathon \_ Fuel Network 33488 - \[Smart Contract - Medium] Insecure implementation of StorageMap c Submitted on Sun Jul 21 2024 20:51:55 GMT-0400 (Atlantic Standard Time) by @jecikpo for [Attackathon | Fuel Network](https://immunefi.com/bounty/fuel-network-attackathon/) Report ID: #33488 Report type: Smart Contract Report severity: Medium Target: Impacts: * Direct theft of any user NFTs, whether at-rest or in-motion, other than unclaimed royalties * Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield * Security Best Practices * Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol) ## Description ## Brief/Intro Currently `StorageMap` implementation could lead to unintended storage slot overwrites if misused by the developer. `StorageMap` uses hashing mechanism (through the usage of `field_id` provided by the developer) to create unique storage slot values by hashing the key through the `field_id`. Nothing however prevents from reusing the `field_id` in a different `StorageMap` instance and hence having the two `StorageMap` instances collide. ## Vulnerability Details Sway offer a `StorageMap` facility for secure key-value data writing into contract's storage. It uses the `field_id` to hash the key and create a unique storage slot value. This works correctly when the developer ensures the uniqueness of the `field_id`. In the future however when the ecosystem grows and dapps are composed of multiple different libraries written by different parties, this will become harder to ensure. ## Impact Details While this is not strictly a bug in the code, it is a faulty library design which might have consequences for future projects. In the event of `StorageMap` collision the impact can be severe as the storage overwritten could result in users assets stolen, as the `StorageMap` shall be used to store the state of user's interaction with the dapp. An example would be a vault which uses `StorageMap` to record users's deposits. Another overlapping `StorageMap` could then be leveraged to create mallicious entries which would then be read by the first `StorageMap` and hence fake deposits could be created. The security of `StorageMap` relies heavily on correctness of the implementation and as Fuel demonstrated strong intent of enforcing correctness of implementation (e.g. through the CEI verification during compile time), I think it is correct to report this under High severity. ## Solution Proposal I created a draft of `SecureStorageMap` implementation which is based on how Solidity creates unique slots for multiple `mappings`. It's description along with code can be found here for reference: ## References The current implementation: ## Proof of concept ## Proof of Concept PoC along with description can be found here: