Skip to content
Merged

feat #573

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { detectMissingOwnershipTransfer } from '../detect-missing-ownership-transfer';

describe('detectMissingOwnershipTransfer', () => {
it('detects direct ownership assignment without two-step pattern', () => {
const code = `
function setOwner(address newOwner) external {
owner = newOwner;
}
`;
const result = detectMissingOwnershipTransfer(code);
expect(result.detected).toBe(true);
expect(result.issues.some(i => i.type === 'missing-two-step')).toBe(true);
});

it('passes when two-step pattern exists', () => {
const code = `
address public pendingOwner;
function proposeOwner(address newOwner) external {
pendingOwner = newOwner;
}
function acceptOwner() external {
owner = pendingOwner;
pendingOwner = address(0);
}
`;
const result = detectMissingOwnershipTransfer(code);
expect(result.detected).toBe(false);
});

it('detects missing zero-address check on transfer', () => {
const code = `
function transferOwnership(address newOwner) external {
owner = newOwner;
}
`;
const result = detectMissingOwnershipTransfer(code);
expect(result.detected).toBe(true);
expect(result.issues.some(i => i.type === 'missing-two-step')).toBe(true);
});

it('detects transferOwnership pattern', () => {
const code = `
function transferOwnership(address newOwner) external {
require(newOwner != address(0));
owner = newOwner;
}
`;
const result = detectMissingOwnershipTransfer(code);
expect(result.detected).toBe(true);
});

it('handles safe code without ownership changes', () => {
const code = `
function getName() external view returns (string memory) {
return name;
}
`;
const result = detectMissingOwnershipTransfer(code);
expect(result.detected).toBe(false);
});
});
81 changes: 81 additions & 0 deletions rules/security/access-control/detect-missing-ownership-transfer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
export interface OwnershipTransferIssue {
type: 'direct-assignment' | 'missing-two-step' | 'missing-zero-check';
line: number;
description: string;
suggestion: string;
}

export interface OwnershipTransferResult {
detected: boolean;
issues: OwnershipTransferIssue[];
message: string;
}

const OWNERSHIP_PATTERNS = [
/(admin|owner|governance|manager)\s*=\s*(\w+)/gi,
/transferOwnership\s*\(\s*(\w+)\s*\)/g,
/setAdmin\s*\(\s*(\w+)\s*\)/g,
/changeAdmin\s*\(\s*(\w+)\s*\)/g,
];

const TWO_STEP_PATTERNS = [
/propose(?:Owner|Admin|Manager)/,
/accept(?:Owner|Admin|Manager)/,
/claim(?:Owner|Admin|Manager)/,
/pending(?:Owner|Admin|Manager)/,
];

const ZERO_ADDRESS_PATTERN = /address\s*\(\s*0\s*\)|0x0+\s*$/;

export function detectMissingOwnershipTransfer(code: string): OwnershipTransferResult {
const issues: OwnershipTransferIssue[] = [];
const lines = code.split('\n');
const codeLower = code.toLowerCase();

const hasTwoStep = TWO_STEP_PATTERNS.some(p => p.test(code));
const hasZeroCheck = ZERO_ADDRESS_PATTERN.test(code);

for (let i = 0; i < lines.length; i++) {
const line = lines[i];
const lineNum = i + 1;
const trimmed = line.trim();

for (const pattern of OWNERSHIP_PATTERNS) {
const matches = trimmed.matchAll(pattern);
for (const match of matches) {
const target = match[1];
const value = match[2];

if (value === 'msg.sender') continue;

if (!hasTwoStep) {
issues.push({
type: 'missing-two-step',
line: lineNum,
description: `Direct ownership transfer of \`${target}\` to \`${value}\` on line ${lineNum} without two-step pattern.`,
suggestion: 'Use a two-step ownership transfer: first propose the new owner, then have them accept.',
});
}

if (!hasZeroCheck && /owner|admin/i.test(target)) {
issues.push({
type: 'missing-zero-check',
line: lineNum,
description: `Ownership transfer of \`${target}\` does not validate the zero address.`,
suggestion: 'Add a require statement to prevent transferring ownership to the zero address.',
});
}
}
}
}

if (issues.length === 0) {
return { detected: false, issues: [], message: 'No unsafe ownership transfer patterns detected.' };
}

return {
detected: true,
issues,
message: `${issues.length} unsafe ownership transfer pattern(s) detected.`,
};
}
Loading