N4A Bounty Program Security Review

Description

Problem The protocol allows the creation of "empty" programs (no payout schedule), but the approval logic expects one. This creates a "dead-end" workflow. Logic Breakdown Creation: _storePayoutSchedule allows severityCount == 0, resulting in hasPayoutSchedule = false. Validation: approvePrimary contains: require(registry.hasPayoutSchedule(programId), "SCHEDULE_REQUIRED");.

Impact

A company can successfully launch a program and researchers can submit work. However, when a judge attempts to approve a report, the transaction will always revert. The program is essentially a trap where work is performed but cannot be processed through the standard primary approval path.

Recommendation

Enforce that a payout schedule with at least one severity must be provided during the program initialization phase in the registry.

Description

The PayoutController::approveSecondOpinion() function constrains the secondary judge to three outcomes: confirm the primary severity (outcome == 0), downgrade it (outcome == 1), or invalidate the finding entirely (outcome == 2). There is no outcome that allows the secondary judge to upgrade the severity above what the primary judge assigned. // outcome 0 -- CONFIRM: severity must exactly match primary if (outcome == 0) { require(severity == report.primarySeverity , " SEVERITY_MISMATCH "); } // outcome 1 -- DOWNGRADE: severity must be strictly less than primary else if (outcome == 1) { require(severity > 0, " INVALID_SEVERITY "); // ... require(severity < report.primarySeverity , " SEVERITY_NOT_DOWNGRADE "); } // outcome 2 -- INVALIDATE: severity must be 0 else if (outcome == 2) { require(severity == 0, " INVALIDATE_SEVERITY "); } If the secondary judge genuinely believes the finding is a higher severity than the primary judge assessed, their only option is to confirm at the primary severity — they cannot express their honest judgment. This is particularly damaging when combined with the fact that escalation and second opinion are mutually exclusive: PayoutController::requestSecondOpinion() requires APPROVED_PRIMARY status, and PayoutController::escalateReport() transitions the report to ESCALATED. Whichever party acts first during the timelock window locks out the other. If the company front-runs the researcher’s escalation by requesting a second opinion, the researcher permanently loses their ability to dispute upward, and the second judge is simultaneously prevented from correcting the severity upward. Note: This is also not a particulary fair system, as when a researcher escelates a report the second judge is free to confirm, upgrade or downgrade the severity. But as mentioned in the case of a second opinion request by a team, the severity can only be confirmed or reduced by the second judge.

Attack Path 1. A researcher submits a report. The primary judge approves it at severity 2 (Medium). The report enters APPROVED_PRIMARY with a timelock window. 2. The researcher believes the finding is Critical (severity 4) and intends to escalate. 3. The company calls requestSecondOpinion() first — the report status moves to SECOND_OPINION_REQUESTED. 4. The researcher can no longer call escalateReport() because the report is no longer in APPROVED_PRIMARY or REJECTED status. 5. The admin assigns a secondary judge. The secondary judge reviews the report and believes the finding is Critical. 6. The secondary judge cannot set severity 4. Their only options are: confirm Medium (outcome 0), downgrade to Low (outcome 1), or invalidate (outcome 2). 7. The secondary judge is forced to confirm at Medium — the researcher receives a Medium payout for a Critical finding.

Root Cause PayoutController::approveSecondOpinion() only defines three outcome paths (confirm, downgrade, invalidate) with no upgrade path. The PayoutController::requestSecondOpinion() function requires Status.APPROVED_PRIMARY, while PayoutController::escalateReport() also requires APPROVED_PRIMARY or REJECTED — making them mutually exclusive once either is called. The ReportManager::markSecondOpinionRequested() does accept ESCALATED status, suggesting the original design may have intended to allow both flows, but PayoutController::requestSecondOpinion() does not use that path.

Impact

• The secondary judge cannot give an honest assessment when they believe severity should be higher, undermining the integrity of the dispute resolution system. • A company can strategically front-run a researcher’s escalation by requesting a second opinion first, knowing the outcome can only stay the same or go lower. • Researchers can be permanently underpaid for legitimate high-severity findings with no remaining recourse — they cannot escalate, and the second judge cannot upgrade on their behalf.

Recommendation

Add an UPGRADE outcome (outcome == 3) to approveSecondOpinion() that allows the secondary judge to set a severity higher than the primary: function approveSecondOpinion ( uint256 programId , address researcher , bytes32 reportHash , uint8 severity , uint8 outcome ) external nonReentrant { require(judges.isJudge(msg.sender), "NOT_JUDGE"); require(researcher != address (0), " RESEARCHER_ZERO "); require(reportHash != bytes32 (0), "HASH_ZERO"); - require(outcome == 0 || outcome == 1 || outcome == 2, " INVALID_OUTCOME"); + require(outcome <= 3, " INVALID_OUTCOME "); // ... existing config/status checks ... if (outcome == 0) { require(severity == report.primarySeverity , " SEVERITY_MISMATCH "); payoutAmount = report.payoutAmount; companyPenaltyBps = config. companyPenaltyBps ; } else if (outcome == 1) { // ... existing downgrade logic ... } else if (outcome == 2) { require(severity == 0, " INVALIDATE_SEVERITY "); judgePenaltyBps = config. judgePenaltyInvalidBps ; payoutAmount = 0; + } else if (outcome == 3) { + require(severity > report.primarySeverity , " SEVERITY_NOT_UPGRADE "); + require(severity < 5, " INVALID_SEVERITY "); + payoutAmount = registry. payoutBySeverity (programId , severity); + require(payoutAmount > 0, " SEVERITY_NOT_CONFIGURED "); + judgePenaltyBps = config. judgePenaltyDowngradeBps ; } Additionally, consider allowing escalateReport() to work on reports in SECOND_OPINION_REQUESTED status, so that both parties can express their disagreement with the primary judgement independently.

Description

The withdraw function sends the entire contract balance to the admin and then emits Withdrawn with address(this).balance as the amount. Because the ETH transfer executes before the event emission, address(this).balance is already zero at the point the event is emitted. Every Withdrawn event will log an amount of zero regardless of how much ETH was actually withdrawn.

Root Cause The balance is read after the transfer rather than before it: function withdraw () external onlyAdmin { (bool ok ,) = payable(msg.sender).call{value: address(this).balance }(""); require(ok , " ETH_TRANSFER_FAILED "); emit Withdrawn(msg.sender , address(this).balance); // balance is now 0 }

Impact

Off-chain systems, monitoring dashboards, and audit trails that rely on events to track fund flows will always see a withdrawn amount of zero. This makes the event log unreliable for accounting purposes.

Recommendation

Capture the balance before the transfer and use the cached value in the event: function withdraw () external onlyAdmin { uint256 amount = address(this).balance; (bool ok ,) = payable(msg.sender).call{value: amount }(""); require(ok , " ETH_TRANSFER_FAILED "); emit Withdrawn(msg.sender , amount); }