Hash collision #58
Hash collision #58
Conversation
There was a problem hiding this comment.
Good one! If we wanna make the vuln focused on hash collisions I think would be good to include other possible ways for them to occur. Otherwise it could be good to just make the vuln focus on hash collisions due to variable length arguments with encodePacked
vulnerabilities/hash-collision.md
Outdated
|
|
||
| ## Understanding the vulnerability | ||
|
|
||
| When `abi.encodePacked()` is used with multiple variable-length arguments (such as arrays), the packed encoding does not include information about the boundaries between different arguments. This can lead to situations where different combinations of arguments result in the same encoded output, causing hash collisions. |
There was a problem hiding this comment.
Should also note that strings fall under this umbrella which seems to be lesser known
vulnerabilities/hash-collision.md
Outdated
|
|
||
| To prevent this type of hash collision, the below remediation strategies can be employed: | ||
|
|
||
| 1. **Avoid Variable-Length Arguments**: Avoid using `abi.encodePacked()` with variable-length arguments. Instead, use fixed-length arrays to ensure the encoding is unique and unambiguous. |
There was a problem hiding this comment.
We should also indicate here that strings should be avoided with encodePacked
vulnerabilities/hash-collision.md
Outdated
| ## Key Concepts | ||
|
|
||
| - **Hash Collision**: A situation where two different sets of inputs produce the same hash output. In this context, a hash collision can occur when using `abi.encodePacked()` with multiple variable-length arguments, allowing an attacker to craft different inputs that produce the same hash. | ||
| - **Signature Verification**: A common method for authentication and authorization in smart contracts, where a message signed by a private key is verified using the corresponding public key. |
There was a problem hiding this comment.
I'm not clear how this plays into this vuln
There was a problem hiding this comment.
I do agree that with it included, the text looks like a blogpost explaining what seems like common knowledge.
I put it in there for context to cater for devs, junior security researchers and generally people who may not be that familiar with above said concepts.
There was a problem hiding this comment.
Not sure if it's clear here but I'm just referring to "Signature Verification" being included here because I don't actually think it's a relevant concept for this vulnerability
Regarding this @kadenzipfel, it would be better to just focus on a single instance of hash collisions and publish a new one when another instance of hash collision is found. Like what is done with DoS vulnerabilities. All situations that can lead to DoS are not in one file but rather separate. Cause I am thinking if we do what you suggest with hash collisions, then we have to do the same for DoS for consistency :) |
|
How is it looking now? |
Related Issue
Checklist
Describe the changes you've made:
Addition of a vulnerability shading light on the dangers of using
abi.encodePacked()with multiple variable-length arguments that could potentially led to a hash collision. This specific vulnerability can lead to security issues in smart contracts, particularly in signature verification scenarios, allowing attackers to bypass authorization mechanisms.The added information includes:
abi.encodePacked()can lead to hash collisions and the potential impact on smart contract security.abi.encode()instead ofabi.encodePacked()and using fixed-length arrays.Type of change