Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .semgrep/rules/sol-rules.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -421,3 +421,37 @@ rules:
paths:
include:
- packages/contracts-bedrock/src/L1/opcm/OPContractsManagerV2.sol

- id: sol-safety-initialize-assert-proxy-admin
languages: [solidity]
severity: ERROR
message: Initialize functions must call _assertOnlyProxyAdminOrProxyAdminOwner() to restrict initialization to authorized callers
patterns:
- pattern: |
function initialize(...) {
...
}
- pattern-not: |
function initialize(...) {
...
_assertOnlyProxyAdminOrProxyAdminOwner();
...
}
paths:
include:
- packages/contracts-bedrock/src
exclude:
# TODO(#19001): We should eventually have this on the L2 contracts.
- packages/contracts-bedrock/src/L2
- packages/contracts-bedrock/src/universal/OptimismMintableERC20Factory.sol
# Dispute games use initialize() in a different context - they are clone-deployed
# via DisputeGameFactory and use their own AlreadyInitialized check pattern.
- packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol
- packages/contracts-bedrock/src/dispute/PermissionedDisputeGame.sol
- packages/contracts-bedrock/src/dispute/SuperFaultDisputeGame.sol
- packages/contracts-bedrock/src/dispute/SuperPermissionedDisputeGame.sol
- packages/contracts-bedrock/src/dispute/zk/OptimisticZkGame.sol
# ProtocolVersions is being deprecated.
- packages/contracts-bedrock/src/L1/ProtocolVersions.sol
# DataAvailabilityChallenge is a beta/non-standard contract.
- packages/contracts-bedrock/src/L1/DataAvailabilityChallenge.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Semgrep tests for Solidity rules are defined in this file.
// Semgrep tests do not need to be valid Solidity code but should be syntactically correct so that
// Semgrep can parse them. You don't need to be able to *run* the code here but it should look like
// the code that you expect to catch with the rule.
//
// Semgrep testing 101
// Use comments like "ruleid: <rule-id>" to assert that the rule catches the code.
// Use comments like "ok: <rule-id>" to assert that the rule does not catch the code.

contract SemgrepTest__sol_safety_initialize_assert_proxy_admin {
// ok: sol-safety-initialize-assert-proxy-admin
function initialize(address _param) external reinitializer(1) {
_assertOnlyProxyAdminOrProxyAdminOwner();
_setParam(_param);
}

// ok: sol-safety-initialize-assert-proxy-admin
function initialize() external initializer {
_assertOnlyProxyAdminOrProxyAdminOwner();
}

// ok: sol-safety-initialize-assert-proxy-admin
function initialize(address _a, uint256 _b) public reinitializer(2) {
// Comment before assertion
_assertOnlyProxyAdminOrProxyAdminOwner();
// More initialization
param = _a;
}

// ruleid: sol-safety-initialize-assert-proxy-admin
function initialize(address _param) external reinitializer(1) {
_setParam(_param);
}

// ruleid: sol-safety-initialize-assert-proxy-admin
function initialize() external initializer {
__Ownable_init();
}

// ruleid: sol-safety-initialize-assert-proxy-admin
function initialize(address _a, uint256 _b) public initializer {
param = _a;
value = _b;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,13 @@
contract SemgrepTest__sol_safety_proper_initializer {
// ok: sol-safety-proper-initializer
function initialize() external {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ok: sol-safety-proper-initializer
function initialize() public {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}
}
Expand All @@ -29,36 +31,43 @@ contract SemgrepTest__sol_safety_proper_initializer {
contract SemgrepTest__sol_safety_proper_initializer {
// ok: sol-safety-proper-initializer
function initialize() external initializer {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ok: sol-safety-proper-initializer
function initialize() public initializer {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ok: sol-safety-proper-initializer
function initialize() external reinitializer(1) {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ok: sol-safety-proper-initializer
function initialize() external reinitializer(1) {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ok: sol-safety-proper-initializer
function initialize() public reinitializer(2) {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ruleid: sol-safety-proper-initializer
function initialize() internal {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}

// ruleid: sol-safety-proper-initializer
function initialize() public {
_assertOnlyProxyAdminOrProxyAdminOwner();
// ...
}
}