Malicious actor can DoS admin functions due to lack of access control
*issue-contents*
0 CONTENTS
- 1................................................................................
- 2................................................................................
- 3................................................................................
- 4................................................................................
- 5................................................................................
- 6................................................................................
- 7................................................................................
- 8................................................................................
- 9................................................................................
1 METADATA
Number | 521 |
---|---|
Severity | Medium |
Author | tjonair |
Contest | Autonomint |
Platform | Sherlock |
2 SUMMARY
The
multiSign::approveSetterFunction
is used by owners to approve specific setter functions of their choice which
allow the admin to set critical admin functions in borrowing
and CDS
contracts.
function approveSetterFunction(
uint8[] memory functions
) external onlyOwners {
The
multiSign::executeSetterFunction
is used by the admin functions of borrowing
and CDS
contracts to check the
approval first to allow updating certain functionalities as well as set the
approvals back to false.
// Example of how this function is being called in both contracts
if (!multiSign.executeSetterFunction(IMultiSign.SetterFunctions(2))) revert CDS_RequiredApprovalsNotMetToSet();
However, this function is not restricted to borrowing
and CDS
contracts due
to lack of access control.
function executeSetterFunction(
SetterFunctions _function
) external returns (bool) { <@ - // Lack of access control
// Get the number of approvals
require(getSetterFunctionApproval(_function) >= requiredApprovals,"Required approvals not met");
// Loop through the approved functions with owners mapping and change to false
for (uint64 i; i < noOfOwners; i++) {
approvedToUpdate[_function][owners[i]] = false;
}
return true;
}
Hence, an attacker can deny the admin from updating critical admin
functionalities in borrowing
and CDS
contracts.
3 ROOT CAUSE
In
multiSign.sol:306
,
there is lack of access control which would allow anyone to call
executeSetterFunction
and reset the approvedToUpdate
values to false.
4 INTERNAL PRE-CONDITIONS
No response
5 EXTERNAL PRE-CONDITIONS
No response
6 ATTACK PATH
- Any of the owners in the
multiSign
contract callsapproveSetterFunction
to provide approvals for updation of a certain function. - Malicious actor keeps on checking the values of
approvedToUpdate
for all owners (which will be less in number) and at any time if it turns out to be true, he can call theexecuteSetterFunction
to deny the owner from calling update functions inCDS
andborrowing
contracts.
7 IMPACT
- DoS of a plethora of admin functions in both
CDS
andborrowing
contracts.
8 POC
Add the following test inside the BorrowingTest.ts
:
describe("Testing Admin functions", async () => {
it("Should allow anyone to set all approvedToUpdate to false", async () => {
const { multiSignA } = await loadFixture(deployer);
// Owner1 and Owner2 approve to update 1 and 2
await multiSignA.connect(owner1).approveSetterFunction([1, 2, 0]);
await multiSignA.connect(owner2).approveSetterFunction([1, 2, 0]);
expect(
await multiSignA.approvedToUpdate(1, await owner1.getAddress())
).to.be.equal(true);
expect(
await multiSignA.approvedToUpdate(2, await owner2.getAddress())
).to.be.equal(true);
// Anyone can set all approvedToUpdate to false
await multiSignA.connect(user1).executeSetterFunction(1);
await multiSignA.connect(user1).executeSetterFunction(2);
expect(
await multiSignA.approvedToUpdate(1, await owner1.getAddress())
).to.be.equal(false);
expect(
await multiSignA.approvedToUpdate(2, await owner2.getAddress())
).to.be.equal(false);
});
});
9 MITIGATION
It is recommended to consider adding a onlyCoreContracts
modifier to
executeSetterFunction
.
LINKS
*issue-links*
- 1.
- 2.
- 3.