Please note, this content is no longer actively maintained.
The content of the SWC registry has not been thoroughly updated since 2020. It is known to be incomplete and may contain errors as well as crucial omissions.
For currently maintained guidance on known Smart Contract vulnerabilities written primarily as guidance for security reviewers, please see the EEA EthTrust Security Levels specification. As well as the latest release version, an Editor's draft is available, that represents the latest work of the group developing the specification.
General guidance for developers on what to consider to ensure security, that is currently maintained, is also available through the Smart Contract Security Verification Standard (SCSVS).
Title
Hash Collisions With Multiple Variable Length Arguments
Relationships
CWE-294: Authentication Bypass by Capture-replay
Description
Using abi.encodePacked()
with multiple variable length arguments can, in certain situations, lead to a hash collision. Since abi.encodePacked()
packs all elements in order regardless of whether they're part of an array, you can move elements between arrays and, so long as all elements are in the same order, it will return the same encoding. In a signature verification situation, an attacker could exploit this by modifying the position of elements in a previous function call to effectively bypass authorization.
Remediation
When using abi.encodePacked()
, it's crucial to ensure that a matching signature cannot be achieved using different parameters. To do so, either do not allow users access to parameters used in abi.encodePacked()
, or use fixed length arrays. Alternatively, you can simply use abi.encode()
instead.
It is also recommended that you use replay protection (see SWC-121), although an attacker can still bypass this by front-running.
References
Samples
access_control.sol
/*
* @author: Steve Marx
*/
pragma solidity ^0.5.0;
import "./ECDSA.sol";
contract AccessControl {
using ECDSA for bytes32;
mapping(address => bool) isAdmin;
mapping(address => bool) isRegularUser;
// Add admins and regular users.
function addUsers(
address[] calldata admins,
address[] calldata regularUsers,
bytes calldata signature
)
external
{
if (!isAdmin[msg.sender]) {
// Allow calls to be relayed with an admin's signature.
bytes32 hash = keccak256(abi.encodePacked(admins, regularUsers));
address signer = hash.toEthSignedMessageHash().recover(signature);
require(isAdmin[signer], "Only admins can add users.");
}
for (uint256 i = 0; i < admins.length; i++) {
isAdmin[admins[i]] = true;
}
for (uint256 i = 0; i < regularUsers.length; i++) {
isRegularUser[regularUsers[i]] = true;
}
}
}
access_control_fixed_1.sol
/*
* @author: Steve Marx
* Modified by Kaden Zipfel
*/
pragma solidity ^0.5.0;
import "./ECDSA.sol";
contract AccessControl {
using ECDSA for bytes32;
mapping(address => bool) isAdmin;
mapping(address => bool) isRegularUser;
// Add a single user, either an admin or regular user.
function addUser(
address user,
bool admin,
bytes calldata signature
)
external
{
if (!isAdmin[msg.sender]) {
// Allow calls to be relayed with an admin's signature.
bytes32 hash = keccak256(abi.encodePacked(user));
address signer = hash.toEthSignedMessageHash().recover(signature);
require(isAdmin[signer], "Only admins can add users.");
}
if (admin) {
isAdmin[user] = true;
} else {
isRegularUser[user] = true;
}
}
}
access_control_fixed_2.sol
/*
* @author: Steve Marx
* Modified by Kaden Zipfel
*/
pragma solidity ^0.5.0;
import "./ECDSA.sol";
contract AccessControl {
using ECDSA for bytes32;
mapping(address => bool) isAdmin;
mapping(address => bool) isRegularUser;
// Add admins and regular users.
function addUsers(
// Use fixed length arrays.
address[3] calldata admins,
address[3] calldata regularUsers,
bytes calldata signature
)
external
{
if (!isAdmin[msg.sender]) {
// Allow calls to be relayed with an admin's signature.
bytes32 hash = keccak256(abi.encodePacked(admins, regularUsers));
address signer = hash.toEthSignedMessageHash().recover(signature);
require(isAdmin[signer], "Only admins can add users.");
}
for (uint256 i = 0; i < admins.length; i++) {
isAdmin[admins[i]] = true;
}
for (uint256 i = 0; i < regularUsers.length; i++) {
isRegularUser[regularUsers[i]] = true;
}
}
}