First Flight #2: Puppy Raffle

Summary

Lack of access control for critical functionality

Vulnerability Details

1. function selectWinner() external => can be called by anyone while this is good to ensure anyone can make sure there is a winner, the function relies on msg.sender for the winner selection rarity.

2. withdrawFees => can be called by anyone. This is risky as it may be called at time when it is no opportune e.g owner mistakenly changed feeAddress to wrong account that immediately calls withdrawFees

Impact

Leads to problems such as manipulation of functions e.g msg.sender in selecting winner or fees can be lost to wrong account or sent at inappropriate timing than that preferred by owner

Tools Used

Manual Analysis

Recommendations

Recommended these functions be called by the owner especially withdrawFees as owner has incentive. However selectWinner() is debatable as it adds centralization risk that owner may not want to call it to not pay out
Recommended to add onlyOwner modifier to withdrawFees function