Skip to content

test(audit-low): coverage sweep 2 — 15 LOW test findings in one pass #232

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.

Sign up for GitHub

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

Closed
hardyjosh wants to merge 1 commit into from
Closed

test(audit-low): coverage sweep 2 — 15 LOW test findings in one pass #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 46 additions & 18 deletions test/src/concrete/ProdFork.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ import {PythOracleAdapter} from "st0x.oracle/concrete/oracle/PythOracleAdapter.s
import {MultiPythOracleAdapter} from "st0x.oracle/concrete/oracle/MultiPythOracleAdapter.sol";
import {MorphoProtocolAdapter} from "st0x.oracle/concrete/protocol/MorphoProtocolAdapter.sol";
import {PassthroughProtocolAdapter} from "st0x.oracle/concrete/protocol/PassthroughProtocolAdapter.sol";
import {OracleRegistry} from "st0x.oracle/concrete/registry/OracleRegistry.sol";
import {OracleRegistry, ZeroOracle, ArrayLengthMismatch} from "st0x.oracle/concrete/registry/OracleRegistry.sol";
import {LibProdDeploy} from "st0x.oracle/lib/LibProdDeploy.sol";
import {OraclePausedManual} from "st0x.oracle/abstract/BasePythOracleAdapter.sol";
import {OnlyAdmin, ZeroVault} from "st0x.oracle/lib/LibOracleErrors.sol";

/// @title LibProdOracles
/// @notice Hardcoded production oracle addresses deployed via
Expand Down Expand Up @@ -179,8 +181,24 @@ contract ProdForkTest is Test {
vm.prank(oracleAdmin);
oracle.setPaused(true);

vm.expectRevert();
oracle.latestAnswer();
// Accept either the legacy `OraclePaused()` selector (deployed mainnet
// bytecode pre-corp-actions upgrade) or the post-rename
// `OraclePausedManual()` (post-upgrade). Once the corp-actions stack
// is deployed and mainnet bytecode reflects the rename, this can
// tighten to the new selector only. Tracked via the mainnet redeploy
// tasks (RAI-327 / RAI-328).
(bool ok, bytes memory data) = address(oracle).staticcall(abi.encodeWithSelector(oracle.latestAnswer.selector));
assertFalse(ok, "latestAnswer must revert while paused");
require(data.length >= 4, "paused oracle must revert with a selector");
bytes4 selector;
assembly {
selector := mload(add(data, 0x20))
}
bytes4 legacy = bytes4(keccak256("OraclePaused()"));
assertTrue(
selector == OraclePausedManual.selector || selector == legacy,
"expected OraclePausedManual (new) or OraclePaused (legacy)"
);

// Unpause and verify it works again
vm.prank(oracleAdmin);
Expand All @@ -197,7 +215,7 @@ contract ProdForkTest is Test {
PythOracleAdapter oracle = PythOracleAdapter(LibProdOracles.WTCOIN_ORACLE);

vm.prank(address(0xdead));
vm.expectRevert();
vm.expectRevert(OnlyAdmin.selector);
oracle.setPaused(true);
}

Expand Down Expand Up @@ -357,7 +375,7 @@ contract ProdForkTest is Test {
OracleRegistry registry = OracleRegistry(LibProdDeploy.ORACLE_REGISTRY);

vm.prank(address(0xdead));
vm.expectRevert();
vm.expectRevert(OnlyAdmin.selector);
registry.setOracle(
address(0x1111111111111111111111111111111111111111),
AggregatorV2V3Interface(address(0x2222222222222222222222222222222222222222))
Expand All @@ -376,7 +394,7 @@ contract ProdForkTest is Test {
oracles[0] = AggregatorV2V3Interface(address(0x1111111111111111111111111111111111111111));

vm.prank(address(0xdead));
vm.expectRevert();
vm.expectRevert(OnlyAdmin.selector);
registry.setOracleBulk(vaults, oracles);
}

Expand All @@ -394,7 +412,7 @@ contract ProdForkTest is Test {
oracles[0] = AggregatorV2V3Interface(address(0x1111111111111111111111111111111111111111));

vm.prank(registryAdmin);
vm.expectRevert();
vm.expectRevert(ArrayLengthMismatch.selector);
registry.setOracleBulk(vaults, oracles);
}

Expand All @@ -406,7 +424,7 @@ contract ProdForkTest is Test {
address registryAdmin = registry.admin();

vm.prank(registryAdmin);
vm.expectRevert();
vm.expectRevert(ZeroVault.selector);
registry.setOracle(address(0), AggregatorV2V3Interface(address(0x1111111111111111111111111111111111111111)));
}

Expand All @@ -418,7 +436,7 @@ contract ProdForkTest is Test {
address registryAdmin = registry.admin();

vm.prank(registryAdmin);
vm.expectRevert();
vm.expectRevert(ZeroOracle.selector);
registry.setOracle(address(0x1111111111111111111111111111111111111111), AggregatorV2V3Interface(address(0)));
}

Expand All @@ -443,7 +461,7 @@ contract ProdForkTest is Test {

// Old admin cannot
vm.prank(registryAdmin);
vm.expectRevert();
vm.expectRevert(OnlyAdmin.selector);
registry.setOracle(
address(0x7777777777777777777777777777777777777777),
AggregatorV2V3Interface(address(0x8888888888888888888888888888888888888888))
Expand Down Expand Up @@ -471,12 +489,22 @@ contract ProdForkTest is Test {
vm.prank(registryAdmin);
registry.setOracle(LibProdOracles.WTCOIN_VAULT, AggregatorV2V3Interface(dummyOracle));

// Adapters now point to dummy — calls should revert
vm.expectRevert();
morpho.price();

vm.expectRevert();
passthrough.latestAnswer();
// Adapters now point to dummy — dummy address has no code, so any
// downstream view call into it reverts (either via Solidity's
// abi.decode panic on empty return data, or via an
// `UnexpectedOracleDecimals`-style guard if the adapter checks
// first). We don't pin the exact revert payload — only that the
// call fails — because the framing call (`oracle.decimals()`,
// `oracle.latestAnswer()`, etc.) differs across adapter types
// and across the optional decimal/staleness checks. Use raw
// staticcall rather than `vm.expectRevert(bytes(""))` because
// foundry-nightly panics decoding empty revert data under -vvv
// verbosity (alloy-dyn-abi-1.5.2).
(bool morphoOk,) = address(morpho).staticcall(abi.encodeWithSelector(morpho.price.selector));
assertFalse(morphoOk, "Morpho price must revert when oracle has no code");

(bool passOk,) = address(passthrough).staticcall(abi.encodeWithSelector(passthrough.latestAnswer.selector));
assertFalse(passOk, "Passthrough latestAnswer must revert when oracle has no code");

// Restore original oracle
vm.prank(registryAdmin);
Expand Down Expand Up @@ -505,7 +533,7 @@ contract ProdForkTest is Test {

// Only admin can set registry
vm.prank(address(0xdead));
vm.expectRevert();
vm.expectRevert(OnlyAdmin.selector);
passthrough.setRegistry(currentRegistry);

// Admin can set registry (set to same one, just testing the call works)
Expand Down Expand Up @@ -602,7 +630,7 @@ contract ProdForkTest is Test {
vm.prank(oracleAdmin);
oracle.setPaused(true);

vm.expectRevert();
vm.expectRevert(OraclePausedManual.selector);
oracle.latestAnswer();

vm.prank(oracleAdmin);
Expand Down
6 changes: 3 additions & 3 deletions test/src/concrete/deploy/MorphoProtocolAdapterBeaconSetDeployer.construct.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract BadMorphoImpl {
contract MorphoProtocolAdapterBeaconSetDeployerConstructTest is Test {
function testMorphoProtocolAdapterBeaconSetDeployerConstructZeroImplementation(address initialOwner) external {
vm.assume(initialOwner != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroImplementation.selector));
vm.expectRevert(ZeroImplementation.selector);
new MorphoProtocolAdapterBeaconSetDeployer(
MorphoProtocolAdapterBeaconSetDeployerConfig({
initialOwner: initialOwner, initialMorphoProtocolAdapterImplementation: address(0)
Expand All @@ -48,7 +48,7 @@ contract MorphoProtocolAdapterBeaconSetDeployerConstructTest is Test {
external
{
vm.assume(initialMorphoProtocolAdapterImplementation != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroBeaconOwner.selector));
vm.expectRevert(ZeroBeaconOwner.selector);
new MorphoProtocolAdapterBeaconSetDeployer(
MorphoProtocolAdapterBeaconSetDeployerConfig({
initialOwner: address(0),
Expand Down Expand Up @@ -81,7 +81,7 @@ contract MorphoProtocolAdapterBeaconSetDeployerConstructTest is Test {
})
);

vm.expectRevert(abi.encodeWithSelector(InitializeAdapterFailed.selector));
vm.expectRevert(InitializeAdapterFailed.selector);
deployer.newMorphoProtocolAdapter(OracleRegistry(address(0xCAFE)), address(0xBEEF), address(this));
}

Expand Down
6 changes: 3 additions & 3 deletions test/src/concrete/deploy/MultiPythOracleAdapterBeaconSetDeployer.construct.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract MultiPythOracleAdapterBeaconSetDeployerConstructTest is Test {
/// implementation address is zero.
function testMultiPythOracleAdapterBeaconSetDeployerConstructZeroImplementation(address initialOwner) external {
vm.assume(initialOwner != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroImplementation.selector));
vm.expectRevert(ZeroImplementation.selector);
new MultiPythOracleAdapterBeaconSetDeployer(
MultiPythOracleAdapterBeaconSetDeployerConfig({
initialOwner: initialOwner, initialMultiPythOracleAdapterImplementation: address(0)
Expand All @@ -55,7 +55,7 @@ contract MultiPythOracleAdapterBeaconSetDeployerConstructTest is Test {
external
{
vm.assume(initialMultiPythOracleAdapterImplementation != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroBeaconOwner.selector));
vm.expectRevert(ZeroBeaconOwner.selector);
new MultiPythOracleAdapterBeaconSetDeployer(
MultiPythOracleAdapterBeaconSetDeployerConfig({
initialOwner: address(0),
Expand Down Expand Up @@ -89,7 +89,7 @@ contract MultiPythOracleAdapterBeaconSetDeployerConstructTest is Test {
FeedConfig[] memory feeds = new FeedConfig[](1);
feeds[0] = FeedConfig({priceId: bytes32(uint256(1)), maxAge: 300});

vm.expectRevert(abi.encodeWithSelector(InitializeAdapterFailed.selector));
vm.expectRevert(InitializeAdapterFailed.selector);
deployer.newMultiPythOracleAdapter(
MultiPythOracleAdapterConfig({
vault: address(0xBEEF), feeds: feeds, admin: address(this), pauseConfig: _emptyPauseConfig()
Expand Down
30 changes: 27 additions & 3 deletions test/src/concrete/deploy/OracleRegistryBeaconSetDeployer.construct.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,24 @@ import {
OracleRegistryBeaconSetDeployer,
OracleRegistryBeaconSetDeployerConfig,
ZeroImplementation,
ZeroBeaconOwner
ZeroBeaconOwner,
InitializeRegistryFailed
} from "st0x.oracle/concrete/deploy/OracleRegistryBeaconSetDeployer.sol";

/// @dev Malicious implementation whose `initialize` returns a non-success
/// sentinel, exercising the `InitializeRegistryFailed` branch in
/// `newOracleRegistry`.
contract BadRegistryImpl {
function initialize(bytes calldata) external pure returns (bytes32) {
return bytes32(uint256(0xdead));
}
}

contract OracleRegistryBeaconSetDeployerConstructTest is Test {
/// Test that zero implementation address reverts.
function testConstructZeroImplementation(address initialOwner) external {
vm.assume(initialOwner != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroImplementation.selector));
vm.expectRevert(ZeroImplementation.selector);
new OracleRegistryBeaconSetDeployer(
OracleRegistryBeaconSetDeployerConfig({
initialOwner: initialOwner, initialOracleRegistryImplementation: address(0)
Expand All @@ -26,7 +36,7 @@ contract OracleRegistryBeaconSetDeployerConstructTest is Test {
/// Test that zero beacon owner address reverts.
function testConstructZeroBeaconOwner(address implementation) external {
vm.assume(implementation != address(0));
vm.expectRevert(abi.encodeWithSelector(ZeroBeaconOwner.selector));
vm.expectRevert(ZeroBeaconOwner.selector);
new OracleRegistryBeaconSetDeployer(
OracleRegistryBeaconSetDeployerConfig({
initialOwner: address(0), initialOracleRegistryImplementation: implementation
Expand All @@ -49,6 +59,20 @@ contract OracleRegistryBeaconSetDeployerConstructTest is Test {
assertTrue(address(deployer.I_ORACLE_REGISTRY_BEACON()) != address(0));
}

/// `newOracleRegistry` must revert `InitializeRegistryFailed` when the
/// cloned proxy's `initialize` returns the wrong sentinel. Mirrors the
/// sibling Morpho/MultiPyth pattern; closes audit #56.
function testNewOracleRegistryRevertsInitFailure() external {
BadRegistryImpl bad = new BadRegistryImpl();
OracleRegistryBeaconSetDeployer deployer = new OracleRegistryBeaconSetDeployer(
OracleRegistryBeaconSetDeployerConfig({
initialOwner: address(this), initialOracleRegistryImplementation: address(bad)
})
);
vm.expectRevert(InitializeRegistryFailed.selector);
deployer.newOracleRegistry();
}

/// `Deployment` must carry both `caller` and `oracleRegistry` as indexed
/// topics so indexers can filter by either field. Closes audit #40 / #173.
function testDeploymentEventIsIndexed(address caller) external {
Expand Down
122 changes: 122 additions & 0 deletions test/src/concrete/deploy/OracleUnifiedDeployer.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,126 @@ contract OracleUnifiedDeployerTest is Test {
emit OracleUnifiedDeployer.Deployment(address(this), oracleAdapter, morphoAdapter, passthroughAdapter);
unifiedDeployer.newOracleAndProtocolAdapters(vault, priceId, maxAge, registry, _emptyPauseConfig());
}

/// @dev Packed fuzz inputs for `testOracleUnifiedDeployerPropagatesNonEmptyPauseConfig`.
/// Solidity's stack-depth limit forces grouping the 10+ fuzz vars into a
/// struct so the test body has room for locals + mock setup.
struct PropagateInputs {
address vault;
bytes32 priceId;
uint256 maxAge;
address oracleAdapter;
address morphoAdapter;
address passthroughAdapter;
address registryAdmin;
address corporateActionsVault;
uint64 pauseBefore;
uint64 pauseAfter;
}

/// `OracleUnifiedDeployer.newOracleAndProtocolAdapters` MUST forward the
/// `pauseConfig` argument verbatim into the `PythOracleAdapterConfig` it
/// passes to `PythOracleAdapterBeaconSetDeployer.newPythOracleAdapter`.
/// The existing happy-path test pins only the `_emptyPauseConfig()` shape;
/// a regression that dropped or rewrote the caller's pauseConfig (e.g.
/// substituted an empty one) would pass that test. The `vm.mockCall` /
/// `vm.expectCall` matchers here are keyed on the *non-empty* pauseConfig
/// so the deployer call would revert / fail expectations on any deviation.
/// Closes audit #57.
function testOracleUnifiedDeployerPropagatesNonEmptyPauseConfig(PropagateInputs memory in_) external {
vm.assume(in_.oracleAdapter.code.length == 0);
vm.assume(in_.morphoAdapter.code.length == 0);
vm.assume(in_.passthroughAdapter.code.length == 0);
vm.assume(in_.registryAdmin != address(0));
vm.assume(in_.corporateActionsVault != address(0));

OracleUnifiedDeployer unifiedDeployer = new OracleUnifiedDeployer();
OracleRegistry registry = _createRegistry(in_.registryAdmin);

CorporateActionPauseConfig memory pauseConfig = CorporateActionPauseConfig({
corporateActionsVault: in_.corporateActionsVault,
actionTypeMask: type(uint256).max,
pauseTimeBefore: in_.pauseBefore,
pauseTimeAfter: in_.pauseAfter
});

_armSubDeployerMocksWithPause(in_, registry, pauseConfig);

// `vm.expectCall` is a positive matcher on the exact calldata — if
// the unified deployer drops/rewrites the pauseConfig, this fails.
vm.expectCall(
LibProdDeploy.PYTH_ORACLE_ADAPTER_BEACON_SET_DEPLOYER,
abi.encodeWithSelector(
PythOracleAdapterBeaconSetDeployer.newPythOracleAdapter.selector,
PythOracleAdapterConfig({
vault: in_.vault,
priceId: in_.priceId,
maxAge: in_.maxAge,
admin: address(this),
pauseConfig: pauseConfig
})
)
);

vm.expectEmit();
emit OracleUnifiedDeployer.Deployment(
address(this), in_.oracleAdapter, in_.morphoAdapter, in_.passthroughAdapter
);
unifiedDeployer.newOracleAndProtocolAdapters(in_.vault, in_.priceId, in_.maxAge, registry, pauseConfig);
}

/// @dev Etch + mock every sub-deployer at its prod address. Mocks are
/// keyed on the *non-empty* pauseConfig so a regression that rewrote it
/// in transit would surface as an un-decoded outer revert.
function _armSubDeployerMocksWithPause(
PropagateInputs memory in_,
OracleRegistry registry,
CorporateActionPauseConfig memory pauseConfig
) internal {
vm.etch(LibProdDeploy.PYTH_ORACLE_ADAPTER_BEACON_SET_DEPLOYER, vm.getCode("PythOracleAdapterBeaconSetDeployer"));
vm.mockCall(
LibProdDeploy.PYTH_ORACLE_ADAPTER_BEACON_SET_DEPLOYER,
abi.encodeWithSelector(
PythOracleAdapterBeaconSetDeployer.newPythOracleAdapter.selector,
PythOracleAdapterConfig({
vault: in_.vault,
priceId: in_.priceId,
maxAge: in_.maxAge,
admin: address(this),
pauseConfig: pauseConfig
})
),
abi.encode(in_.oracleAdapter)
);

vm.etch(
LibProdDeploy.MORPHO_PROTOCOL_ADAPTER_BEACON_SET_DEPLOYER,
vm.getCode("MorphoProtocolAdapterBeaconSetDeployer")
);
vm.mockCall(
LibProdDeploy.MORPHO_PROTOCOL_ADAPTER_BEACON_SET_DEPLOYER,
abi.encodeWithSelector(
MorphoProtocolAdapterBeaconSetDeployer.newMorphoProtocolAdapter.selector,
registry,
in_.vault,
address(this)
),
abi.encode(in_.morphoAdapter)
);

vm.etch(
LibProdDeploy.PASSTHROUGH_PROTOCOL_ADAPTER_BEACON_SET_DEPLOYER,
vm.getCode("PassthroughProtocolAdapterBeaconSetDeployer")
);
vm.mockCall(
LibProdDeploy.PASSTHROUGH_PROTOCOL_ADAPTER_BEACON_SET_DEPLOYER,
abi.encodeWithSelector(
PassthroughProtocolAdapterBeaconSetDeployer.newPassthroughProtocolAdapter.selector,
registry,
in_.vault,
address(this)
),
abi.encode(in_.passthroughAdapter)
);
}
}
Loading
Loading