Skip to content

Commit

Permalink
Add support for skipping signatures that aren't from owners (and warn…
Browse files Browse the repository at this point in the history
…ing via console)
  • Loading branch information
mdehoog committed Dec 11, 2024
1 parent c6a9989 commit bf9a741
Showing 1 changed file with 32 additions and 12 deletions.
44 changes: 32 additions & 12 deletions script/universal/Signatures.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.15;
import {Bytes} from "@eth-optimism-bedrock/src/libraries/Bytes.sol";
import {LibSort} from "@solady/utils/LibSort.sol";
import {IGnosisSafe} from "./IGnosisSafe.sol";
import {console} from "forge-std/console.sol";

library Signatures {
function prepareSignatures(address _safe, bytes32 hash, bytes memory _signatures)
Expand All @@ -17,7 +18,9 @@ library Signatures {
_signatures = bytes.concat(prevalidatedSignatures, _signatures);

// safe requires all signatures to be unique, and sorted ascending by public key
return sortUniqueSignatures(_signatures, hash, IGnosisSafe(_safe).getThreshold(), prevalidatedSignatures.length);
return sortUniqueSignatures(
_safe, _signatures, hash, IGnosisSafe(_safe).getThreshold(), prevalidatedSignatures.length
);
}

function genPrevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) {
Expand Down Expand Up @@ -64,6 +67,7 @@ library Signatures {
/**
* @notice Sorts the signatures in ascending order of the signer's address, and removes any duplicates.
* @dev see https://github.com/safe-global/safe-smart-account/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/Safe.sol#L265-L334
* @param _safe Address of the Safe that should verify the signatures.
* @param _signatures Signature data that should be verified.
* Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash.
* Can be suffixed with EIP-1271 signatures after threshold*65 bytes.
Expand All @@ -73,22 +77,23 @@ library Signatures {
* Can be used to accomodate any additional signatures prepended to the array.
* If prevalidated signatures were prepended, this should be the length of those signatures.
*/
function sortUniqueSignatures(bytes memory _signatures, bytes32 dataHash, uint256 threshold, uint256 dynamicOffset)
internal
pure
returns (bytes memory)
{
function sortUniqueSignatures(
address _safe,
bytes memory _signatures,
bytes32 dataHash,
uint256 threshold,
uint256 dynamicOffset
) internal view returns (bytes memory) {
bytes memory sorted;
uint256 count = uint256(_signatures.length / 0x41);
uint256[] memory addressesAndIndexes = new uint256[](threshold);
address[] memory uniqueAddresses = new address[](threshold);
uint8 v;
bytes32 r;
bytes32 s;
uint256 j;
for (uint256 i; i < count; i++) {
(v, r, s) = signatureSplit(_signatures, i);
address owner = extractOwner(dataHash, r, s, v);
(address owner, bool isOwner) = extractOwner(_safe, _signatures, dataHash, i);
if (!isOwner) {
continue;
}

// skip duplicate owners
uint256 k;
Expand All @@ -109,7 +114,7 @@ library Signatures {
LibSort.sort(addressesAndIndexes);
for (uint256 i; i < count; i++) {
uint256 index = addressesAndIndexes[i] & 0xffffffff;
(v, r, s) = signatureSplit(_signatures, index);
(uint8 v, bytes32 r, bytes32 s) = signatureSplit(_signatures, index);
if (v == 0) {
// The `s` value is used by safe as a lookup into the signature bytes.
// Increment by the offset so that the lookup location remains correct.
Expand All @@ -125,6 +130,21 @@ library Signatures {
return sorted;
}

function extractOwner(address _safe, bytes memory _signatures, bytes32 dataHash, uint256 i)
internal
view
returns (address, bool)
{
(uint8 v, bytes32 r, bytes32 s) = signatureSplit(_signatures, i);
address owner = extractOwner(dataHash, r, s, v);
bool isOwner = IGnosisSafe(_safe).isOwner(owner);
if (!isOwner) {
console.log("---\nSkipping the following signature, which was recovered to a non-owner address %s:", owner);
console.logBytes(abi.encodePacked(r, s, v));
}
return (owner, isOwner);
}

function extractOwner(bytes32 dataHash, bytes32 r, bytes32 s, uint8 v) internal pure returns (address) {
if (v <= 1) {
return address(uint160(uint256(r)));
Expand Down

0 comments on commit bf9a741

Please sign in to comment.