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

🛡️ Unauthorized Object Deletion Triggers Denial of Service #551

Closed
ccamel opened this issue May 23, 2024 · 3 comments · Fixed by #588
Closed

🛡️ Unauthorized Object Deletion Triggers Denial of Service #551

ccamel opened this issue May 23, 2024 · 3 comments · Fixed by #588
Assignees
Labels
security audit Categorizes an issue or PR as relevant to Security Audit

Comments

@ccamel
Copy link
Member

ccamel commented May 23, 2024

Note

Severity: High
target: v5.0.0 - Commit: cde785fbd2dad71608d53f8524e0ef8c8f8178af
Ref: OKP4 CosmWasm Audit Report v1.0 - 02-05-2024 - BlockApex

Description

In the Objectarium contract, the store_object function allows users to store data objects with an option to pin them. Pinning an object prevents its deletion unless explicitly unpinned by the owner. However, a issue arrises when objects are stored without being pinned (pin: false). In such cases, any user, regardless of ownership, can invoke the forget_object function to delete these objects.

This flaw arises from the lack of proper ownership verification and permission checks before object deletion. Consequently, this issue allows unauthorized users to delete objects they do not own, leading to potential Denial of Service (DoS) attacks.If a user consistently stores objects with pin set to false, an attacker can monitor these actions and systematically delete newly stored objects by repeatedly calling the forget_object function. This behavior can repeatedly disrupt legitimate users' attempts to
store data, effectively denying them the service of the contract.

Impact

The primary impact of this vulnerability is unauthorized data manipulation leading to Denial of Service. By allowing any user to delete objects they do not own, the system's integrity and reliability are compromised. This not only violates access controls but also exposes the system to targeted attacks where critical data can be maliciously wiped out.

Recommendation

To mitigate this vulnerability, implement stringent ownership checks within the forget_object function to ensure that only the object's owner can delete it. This could involve Verifying that the caller of forget_object matches the stored owner's information before allowing the object's deletion.

@ccamel ccamel added the security audit Categorizes an issue or PR as relevant to Security Audit label May 23, 2024
@amimart
Copy link
Member

amimart commented Jun 13, 2024

In my sense it's the responsibility to the owner to pin the object when storing it, if he rely on it. I'd tend to do nothing here.

Let me know your opinion on that matter @bdeneux @ccamel 🤔

@bdeneux
Copy link
Contributor

bdeneux commented Jun 14, 2024

@amimart, I agree with your perspective. One potential solution could be to add an option during bucket instantiation. This option could specify whether only an admin can forget an object, or even provide a list of addresses that are allowed to do so. However, it's worth considering whether this feature is truly necessary.

@ccamel
Copy link
Member Author

ccamel commented Jun 14, 2024

The initial idea behind this behavior was to create a mechanism akin to IPFS, incorporating the concept of pinning. Non-pinned stored resources can be "recycled" at any time by anyone, whereas pinned stored resources are persisted and can be accessed indefinitely. In the context of our protocol, where all operations are transparent, it is evident that this could become an attack vector, allowing systematic deletion of non-pinned resources at minimal financial cost. Over time, users would likely pin all their resources, rendering the pinning option quite ineffective.

However, at this point, I don’t see an urgent need for changes, provided that this behaviour and its potential consequences are clearly documented. An adjustment in the documentation could address this, which would be my only recommendation. 😌

@amimart @bdeneux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security audit Categorizes an issue or PR as relevant to Security Audit
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants