-
Notifications
You must be signed in to change notification settings - Fork 2
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #6 from Credshields/main
Added 7 missing SCWEs
- Loading branch information
Showing
10 changed files
with
407 additions
and
56 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
--- | ||
title: Improper Use of CREATE2 for Contract Deployment | ||
id: SCWE-051 | ||
alias: improper-create2-deployment | ||
platform: [] | ||
profiles: [L1] | ||
mappings: | ||
scsvs-cg: [SCSVS-ARCH] | ||
scsvs-scg: [SCSVS-ARCH-2] | ||
cwe: [706] | ||
status: new | ||
--- | ||
|
||
## Relationships | ||
- **CWE-706: Use of Incorrectly-Resolved Name or Reference** | ||
[https://cwe.mitre.org/data/definitions/706.html](https://cwe.mitre.org/data/definitions/706.html) | ||
|
||
## Description | ||
The `CREATE2` opcode allows for deterministic contract deployment, meaning the contract's address can be precomputed before deployment. However, improper handling of the salt parameter, constructor arguments, or contract bytecode can lead to vulnerabilities such as address predictability, re-deployment attacks, and malicious contract substitution. If an attacker can influence the salt or code, they may deploy a contract at a known address before the legitimate one, leading to security risks. | ||
|
||
## Remediation | ||
To prevent misuse of `CREATE2`: | ||
- Use unpredictable and unique salt values (e.g., incorporating nonces, sender addresses, or randomness). | ||
- Ensure the deployed contract logic remains consistent to prevent re-deployment attacks. | ||
- Hash important contract parameters into the salt to prevent unintended address collisions. | ||
|
||
### Vulnerable Contract Example | ||
```solidity | ||
contract Factory { | ||
function deploy(bytes32 salt, bytes memory bytecode) public { | ||
address deployed; | ||
assembly { | ||
deployed := create2(0, add(bytecode, 0x20), mload(bytecode), salt) | ||
} | ||
} | ||
} | ||
``` | ||
|
||
**Why is this vulnerable?** | ||
|
||
- If `salt` is predictable (e.g., user-supplied or a static value), an attacker can precompute and front-run the deployment. | ||
- The same contract address can be re-used by deploying different bytecode, leading to logic changes at a fixed address. | ||
- No validation ensures that the deployed contract is safe or expected. | ||
|
||
### Fixed Contract Example | ||
|
||
```solidity | ||
contract SecureFactory { | ||
function deploy(bytes32 salt, bytes memory bytecode) public returns (address) { | ||
require(bytecode.length > 0, "Bytecode cannot be empty"); | ||
// Ensure the salt includes sender information to prevent front-running | ||
bytes32 secureSalt = keccak256(abi.encodePacked(msg.sender, salt)); | ||
address deployed; | ||
assembly { | ||
deployed := create2(0, add(bytecode, 0x20), mload(bytecode), secureSalt) | ||
} | ||
require(deployed != address(0), "Deployment failed"); | ||
return deployed; | ||
} | ||
} | ||
``` | ||
|
||
**Why is this safe?** | ||
- Uses `keccak256(abi.encodePacked(msg.sender, salt))` to make the salt unique per sender. | ||
- Ensures the contract bytecode is non-empty before deploying. | ||
- Validates that the deployed contract is nonzero, ensuring a successful deployment. | ||
|
||
**By securing CREATE2 deployments, developers can prevent predictable contract addresses, front-running risks, and contract replacement vulnerabilities.** | ||
|
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
--- | ||
title: Incorrect Storage Packing | ||
id: SCWE-040 | ||
alias: incorrect-storage-packing | ||
platform: [] | ||
profiles: [L1] | ||
mappings: | ||
scsvs-cg: [SCSVS-CODE] | ||
scsvs-scg: [SCSVS-CODE-1] | ||
cwe: [805] | ||
status: new | ||
--- | ||
|
||
|
||
## Relationships | ||
- **CWE-805: Buffer Access with Incorrect Length Value** | ||
[https://cwe.mitre.org/data/definitions/805.html](https://cwe.mitre.org/data/definitions/805.html) | ||
|
||
## Description | ||
Incorrect storage packing in Solidity occurs when storage variables are not efficiently packed within a single storage slot, leading to unnecessary gas consumption. Solidity automatically stores variables in 32-byte (256-bit) slots, and inefficient ordering of data types can lead to wasted space. Contracts that fail to optimize storage packing may incur higher gas costs during deployments and transactions. | ||
|
||
## Remediation | ||
To optimize storage packing: | ||
- Group smaller data types together (e.g., `uint8`, `bool`) to fit within a single 32-byte slot. | ||
- Avoid leaving gaps between variables of different sizes. | ||
- Order state variables efficiently to minimize wasted storage slots. | ||
|
||
### Vulnerable Contract Example | ||
```solidity | ||
contract InefficientStorage { | ||
uint256 a; // Occupies full 32-byte slot | ||
bool b; // Occupies another 32-byte slot (wasteful) | ||
uint8 c; // Uses a new 32-byte slot instead of sharing | ||
uint256 d; // Uses its own slot, leading to extra gas costs | ||
} | ||
``` | ||
|
||
**Why is this vulnerable?** | ||
- Each variable unnecessarily occupies a separate storage slot, increasing gas costs. | ||
- The `bool` and `uint8` could be packed into the same 32-byte slot, reducing wasted storage space. | ||
|
||
### Fixed Contract Example | ||
|
||
```solidity | ||
contract OptimizedStorage { | ||
uint256 a; // Occupies one full 32-byte slot | ||
uint256 d; // Placed next to 'a' to use another full slot | ||
bool b; // Packed within the same slot as 'c' | ||
uint8 c; // Fits within the same slot as 'b' | ||
} | ||
``` | ||
**Why is this safe?** | ||
- `bool` and `uint8` share the same storage slot, reducing unnecessary storage use. | ||
- `uint256` variables are placed together to minimize fragmentation. | ||
- Optimized storage layout leads to lower gas costs for contract execution. | ||
|
||
**By correctly ordering state variables and utilizing Solidity’s storage packing rules, developers can significantly reduce gas fees and improve contract efficiency.** |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
--- | ||
title: Unsafe Downcasting | ||
id: SCWE-041 | ||
alias: unsafe-downcasting | ||
platform: [] | ||
profiles: [L1] | ||
mappings: | ||
scsvs-cg: [SCSVS-CODE] | ||
scsvs-scg: [SCSVS-CODE-1] | ||
cwe: [681] | ||
status: new | ||
--- | ||
|
||
## Relationships | ||
- **CWE-681: Incorrect Conversion between Numeric Types** | ||
[https://cwe.mitre.org/data/definitions/681.html](https://cwe.mitre.org/data/definitions/681.html) | ||
|
||
## Description | ||
Unsafe downcasting occurs when a larger integer type is implicitly or explicitly converted to a smaller type, leading to precision loss, unintended value changes, or integer overflows. Solidity does not automatically check for overflow when performing explicit type conversions, making it possible to unintentionally truncate values. | ||
|
||
## Remediation | ||
To prevent unsafe downcasting: | ||
- Always validate that the value fits within the target data type before casting. | ||
- Use safe mathematical libraries like OpenZeppelin's `SafeCast` to ensure proper conversions. | ||
- Avoid unnecessary downcasting unless explicitly needed for gas optimization. | ||
|
||
### Vulnerable Contract Example | ||
```solidity | ||
contract UnsafeDowncasting { | ||
function truncateValue(uint256 largeNumber) public pure returns (uint8) { | ||
return uint8(largeNumber); // ⚠️ Potential data loss if largeNumber > 255 | ||
} | ||
} | ||
``` | ||
|
||
**Why is this vulnerable?** | ||
- If `largeNumber > 255`, the higher bits will be truncated, resulting in unexpected values. | ||
- No validation ensures that `largeNumber` fits within `uint8`, leading to silent failures. | ||
|
||
|
||
### Fixed Contract Example | ||
|
||
```solidity | ||
import "@openzeppelin/contracts/utils/math/SafeCast.sol"; | ||
contract SafeDowncasting { | ||
using SafeCast for uint256; | ||
function safeTruncateValue(uint256 largeNumber) public pure returns (uint8) { | ||
return largeNumber.toUint8(); // ✅ Ensures safe conversion | ||
} | ||
} | ||
``` | ||
**Why is this safe?** | ||
|
||
- Uses OpenZeppelin’s `SafeCast` library to enforce safe downcasting. | ||
- If `largeNumber` exceeds `uint8` limits, the transaction will revert instead of silently truncating. | ||
- Prevents unexpected behavior and potential security vulnerabilities. | ||
|
||
**By properly handling type conversions, developers can avoid integer truncation issues and maintain data integrity in smart contracts.** |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
--- | ||
title: Improper Deletion of Mappings | ||
id: SCWE-053 | ||
alias: improper-mapping-deletion | ||
platform: [] | ||
profiles: [L1] | ||
mappings: | ||
scsvs-cg: [SCSVS-CODE] | ||
scsvs-scg: [SCSVS-CODE-1] | ||
cwe: [459] | ||
status: new | ||
--- | ||
|
||
## Relationships | ||
- **CWE-459: Incomplete Cleanup** | ||
[https://cwe.mitre.org/data/definitions/459.html](https://cwe.mitre.org/data/definitions/459.html) | ||
|
||
## Description | ||
In Solidity, mappings (`mapping(address => uint256)`) do not store key-value pairs in a way that allows them to be iterated over or deleted in a straightforward manner. Deleting a mapping variable does not remove its entries; it only resets the reference to the mapping, leaving stale data accessible if another variable references the same mapping. Improper deletion of mappings can lead to storage inconsistencies, potential access control issues, and unintended behavior. | ||
|
||
## Remediation | ||
- Instead of using `delete mappingVariable;`, explicitly set each key’s value to zero where necessary. | ||
- Consider additional data structures (e.g., arrays or linked lists) to track mapping keys if deletion is required. | ||
- Ensure mapping deletions do not leave residual data that can be accessed unexpectedly. | ||
|
||
### Vulnerable Contract Example | ||
```solidity | ||
contract Example { | ||
mapping(address => uint256) public balances; | ||
function addBalance(address user, uint256 amount) public { | ||
balances[user] = amount; | ||
} | ||
function resetBalances() public { | ||
delete balances; // ❌ This does not clear individual key-value pairs! | ||
} | ||
} | ||
``` | ||
**Why is this vulnerable?** | ||
|
||
- `delete balances;` only resets the storage reference but does not remove key-value pairs. | ||
- If another contract or function still references `balances`, the data remains accessible. | ||
- Unexpected behavior may arise where users assume balances have been erased but can still access the old data. | ||
|
||
|
||
### Fixed Contract Example | ||
|
||
```solidity | ||
contract SecureExample { | ||
mapping(address => uint256) public balances; | ||
address[] private users; | ||
function addBalance(address user, uint256 amount) public { | ||
if (balances[user] == 0) { | ||
users.push(user); | ||
} | ||
balances[user] = amount; | ||
} | ||
function resetBalances() public { | ||
for (uint256 i = 0; i < users.length; i++) { | ||
balances[users[i]] = 0; // ✅ Explicitly clears each entry | ||
} | ||
delete users; // ✅ Resets the tracking array | ||
} | ||
} | ||
``` | ||
|
||
**Why is this safe?** | ||
- Tracks users in an array to allow explicit deletion of each mapping entry. | ||
- Ensures all key-value pairs are properly reset instead of just deleting the mapping reference. | ||
- Prevents residual data from being accessed unintentionally. | ||
|
||
**By properly handling mapping deletions, developers can avoid unintended data persistence and ensure accurate contract state management.** |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
--- | ||
title: Incorrect Handling of Bitwise Operations | ||
id: SCWE-066 | ||
alias: incorrect-bitwise-operations | ||
platform: [] | ||
profiles: [L1] | ||
mappings: | ||
scsvs-cg: [SCSVS-CODE] | ||
scsvs-scg: [SCSVS-CODE-2] | ||
cwe: [682] | ||
status: new | ||
--- | ||
|
||
## Relationships | ||
- **CWE-682: Incorrect Calculation** | ||
[https://cwe.mitre.org/data/definitions/682.html](https://cwe.mitre.org/data/definitions/682.html) | ||
|
||
## Description | ||
Bitwise operations (e.g., `&`, `|`, `^`, `<<`, `>>`) can be efficient alternatives to arithmetic operations but are prone to errors if not used correctly. Misusing bitwise shifts can cause unintended value changes, integer overflows, or precision loss. Solidity lacks native overflow checks for bitwise shifts, making incorrect handling particularly dangerous in financial calculations or cryptographic functions. | ||
|
||
## Remediation | ||
- Ensure bitwise shifts do not exceed the size of the data type (e.g., shifting a `uint8` left by 8+ bits). | ||
- Validate input ranges before performing shifts to prevent overflow or precision loss. | ||
- Avoid unnecessary bitwise operations in financial calculations unless explicitly needed. | ||
|
||
### Vulnerable Contract Example | ||
```solidity | ||
contract Example { | ||
function shiftLeft(uint8 input) public pure returns (uint8) { | ||
return input << 8; // ❌ Shifting beyond `uint8` capacity leads to zero | ||
} | ||
function bitwiseAnd(uint8 a, uint8 b) public pure returns (uint8) { | ||
return a & b; // ❌ Without proper validation, this could lead to unintended masking | ||
} | ||
} | ||
``` | ||
**Why is this vulnerable?** | ||
|
||
- `input << 8` shifts a `uint8` completely out of range, resulting in a value of `0`. | ||
- `a & b` can unintentionally mask critical bits if inputs are not properly validated. | ||
|
||
### Fixed Contract Example | ||
|
||
```solidity | ||
contract SecureExample { | ||
function safeShiftLeft(uint8 input) public pure returns (uint8) { | ||
require(input < 32, "Shift too large"); // ✅ Validate shift range | ||
return input << 2; // ✅ Safe shift within `uint8` bounds | ||
} | ||
function safeBitwiseAnd(uint8 a, uint8 b) public pure returns (uint8) { | ||
require(a > 0 && b > 0, "Invalid input"); // ✅ Ensure inputs are meaningful | ||
return a & b; | ||
} | ||
} | ||
``` | ||
|
||
**Why is this safe?** | ||
|
||
- Restricts shift values to prevent unexpected overflows. | ||
- Ensures bitwise operations do not unintentionally modify values in an unintended way. | ||
- Protects contract logic from potential manipulation through poorly validated inputs. | ||
|
||
**By correctly handling bitwise operations, developers can avoid unintended computation errors and ensure mathematical correctness in smart contracts.** |
Empty file.
Empty file.
Oops, something went wrong.