GivingThanks

First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Wrong param (msg.sender instead of _registry) assign to `GivingThanks:registry`

**Description:** In the constructor of `GivingThanks`, the `registry` is assigned to the `msg.sender` instead of `_registry`,
which blocks `GivingThanks:donate` function since it contains `registry.isVerified(charity)`.
```javascript
constructor(address _registry) ERC721("DonationReceipt", "DRC") {
// @audit-high: wrong param pass in, should be _registry instead of msg.sender
@> registry = CharityRegistry(msg.sender);
owner = msg.sender;
tokenCounter = 0;
}
```
**Impact:** The `GivingThanks:donate` function is blocked
**Proof of Concept:** place the following into `GivingThanks.t.sol`.
```javascript
function testWrongCharityRegistryPassing() public view {
assertNotEq(address(charityContract.registry()), address(registryContract));
}
```
**Recommended Mitigation:**
in `GiveThanks.sol`
```diff
constructor(address _registry) ERC721("DonationReceipt", "DRC") {
// @audit-high: wrong param pass in, should be _registry instead of msg.sender
- registry = CharityRegistry(msg.sender);
+ registry = CharityRegistry(_registry);
owner = msg.sender;
tokenCounter = 0;
}
```
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-bad-registry-set-at-construction

Likelyhood: High, the parameter is not well used and won't be set. Impact: Low, can be changed with the setter and no one will be able to donate to malicious charity.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.