Skip to content

Commit

Permalink
Merge pull request #166 from orcaprotocol/willkim/audit
Browse files Browse the repository at this point in the history
Audit: Low risk
  • Loading branch information
Will Kim authored Oct 24, 2022
2 parents ac5d5de + e710b7b commit e3eb27e
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 2 deletions.
4 changes: 4 additions & 0 deletions contracts/ControllerV1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ contract ControllerV1 is
uint256 expectedPodId,
string memory _imageUrl
) external override {
require(_members.length > 0, "cannot have 0 members");
require(threshold > 0, "threshold must be more than 0");
require(_label != bytes32(0), "label cannot be blank");
require(bytes(_ensString).length > 0, "ensString cannot be empty");
address safe = createSafe(_members, threshold, expectedPodId);

_createPod(
Expand Down
6 changes: 6 additions & 0 deletions contracts/MemberTeller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ contract MemberTeller {

function memberTellerCheck(uint256 podId, bytes memory data) internal {
if (bytes4(data) == ENCODED_SIG_ADD_OWNER) {
// Ensure data is at minimum, the length required for the below logic.
require(data.length >= 24, "incorrect data length");
address mintMember;
assembly {
// shift 0x4 for the sig + 0x20 padding
Expand All @@ -41,6 +43,8 @@ contract MemberTeller {
memberToken.mint(mintMember, podId, getSyncData());
}
if (bytes4(data) == ENCODED_SIG_REMOVE_OWNER) {
// Ensure data is at minimum, the length required for the below logic.
require(data.length >= 44, "incorrect data length");
address burnMember;
assembly {
// note: consecutive addresses are packed into a single memory slot
Expand All @@ -51,6 +55,8 @@ contract MemberTeller {
memberToken.burn(burnMember, podId);
}
if (bytes4(data) == ENCODED_SIG_SWAP_OWNER) {
// Ensure data is at minimum, the length required for the below logic.
require(data.length >= 64, "incorrect data length");
address burnMember;
address mintMember;
assembly {
Expand Down
4 changes: 4 additions & 0 deletions contracts/MemberToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ contract MemberToken is ERC1155Supply, Ownable {

// Note that OpenSea does not currently update contract metadata when this value is changed. - Nov 2021
function setContractURI(string memory newContractURI) public onlyOwner {
require(
bytes(newContractURI).length > 0,
"newContractURI cannot be empty"
);
_contractURI = newContractURI;
}

Expand Down
3 changes: 3 additions & 0 deletions contracts/MultiCreateV1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ contract MultiCreateV1 {
IMemberToken immutable memberToken;

constructor(address _memberToken) {
require(_memberToken != address(0), "member token can't be 0 address");
memberToken = IMemberToken(_memberToken);
}

Expand All @@ -26,7 +27,9 @@ contract MultiCreateV1 {
string[] memory _imageUrls
) public returns (address[] memory) {
uint256 numPods = _thresholds.length;
require(numPods > 0, "can't call with 0 pods");
require(_members.length == numPods, "incorrect members array");
require(_admins.length == numPods, "incorrect admins array");
require(_labels.length == numPods, "incorrect labels array");
require(_ensStrings.length == numPods, "incorrect ensStrings array");
require(_imageUrls.length == numPods, "incorrect imageUrls array");
Expand Down
2 changes: 2 additions & 0 deletions contracts/PermissionManager.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pragma solidity ^0.8.7;

import "openzeppelin-contracts/access/AccessControl.sol";
import "openzeppelin-contracts/utils/Address.sol";

// This contract will be the owner of all other contracts in the ecosystem.
contract PermissionManager is AccessControl {
Expand All @@ -13,6 +14,7 @@ contract PermissionManager is AccessControl {
public
onlyRole(DEFAULT_ADMIN_ROLE)
{
Address.isContract(contractAddress);
// Call uses the context of this contract, so msg.sender of other contracts
// will be this contract.
(bool success, ) = contractAddress.call(data);
Expand Down
5 changes: 5 additions & 0 deletions contracts/SafeTeller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ contract SafeTeller {
address _gnosisMasterAddress,
address _fallbackHanderAddress
) {
require(_proxyFactoryAddress != address(0), "Invalid address");
require(_gnosisMasterAddress != address(0), "Invalid address");
require(_fallbackHanderAddress != address(0), "Invalid address");
proxyFactoryAddress = _proxyFactoryAddress;
gnosisMasterAddress = _gnosisMasterAddress;
fallbackHandlerAddress = _fallbackHanderAddress;
Expand All @@ -62,6 +65,8 @@ contract SafeTeller {
_newSafeTeller
);

require(_newSafeTeller != address(0), "safe teller can't be 0 address");

bool enableSuccess = IGnosisSafe(_safe).execTransactionFromModule(
_safe,
0,
Expand Down
2 changes: 0 additions & 2 deletions contracts/ens/IPodEnsRegistrar.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ interface IPodEnsRegistrar {

function register(bytes32 label, address owner) external;

function deregister(address safe, bytes32 label) external;

function setText(
bytes32 node,
string calldata key,
Expand Down

0 comments on commit e3eb27e

Please sign in to comment.