Skip to content

Commit

Permalink
Merge pull request #754 from clrfund/fix/invalid-pub-key
Browse files Browse the repository at this point in the history
Do not allow invalid MACI public key in publishMessage
  • Loading branch information
yuetloo authored May 20, 2024
2 parents 4e3d72a + 43b161d commit 36743e4
Show file tree
Hide file tree
Showing 75 changed files with 689 additions and 961 deletions.
2 changes: 1 addition & 1 deletion .husky/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ export VITE_RECIPIENT_REGISTRY_TYPE=simple
export VITE_USER_REGISTRY_TYPE=simple
export VITE_WALLET_CONNECT_PROJECT_ID=1

yarn test:format && yarn test:web && yarn test:lint-i18n
yarn test:format && yarn test:common && yarn test:web && yarn test:lint-i18n
6 changes: 3 additions & 3 deletions common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@
},
"dependencies": {
"@openzeppelin/merkle-tree": "^1.0.5",
"ethers": "^6.11.1",
"maci-crypto": "^1.2.0",
"maci-domainobjs": "^1.2.0"
"ethers": "^6.12.1",
"maci-crypto": "1.2.2",
"maci-domainobjs": "1.2.2"
},
"repository": {
"type": "git",
Expand Down
21 changes: 21 additions & 0 deletions common/src/__tests__/keypair.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { expect } from 'chai'
import { Keypair, PubKey } from '../keypair'
import { Wallet, sha256, randomBytes } from 'ethers'

describe('keypair', function () {
for (let i = 0; i < 10; i++) {
it(`should generate key ${i} from seed successfully`, function () {
const wallet = Wallet.createRandom()
const signature = wallet.signMessageSync(randomBytes(32).toString())
const seed = sha256(signature)
const keypair = Keypair.createFromSeed(seed)
expect(keypair.pubKey.serialize()).to.match(/^macipk./)
})
}

it('should throw if pubKey is invalid', () => {
expect(() => {
new PubKey([1n, 1n])
}).to.throw('PubKey not on curve')
})
})
38 changes: 16 additions & 22 deletions common/src/keypair.ts
Original file line number Diff line number Diff line change
@@ -1,37 +1,31 @@
import { keccak256, isBytesLike, concat, toBeArray } from 'ethers'
import { Keypair as MaciKeypair, PrivKey, PubKey } from 'maci-domainobjs'

const SNARK_FIELD_SIZE = BigInt(
'21888242871839275222246405745257275088548364400416034343698204186575808495617'
)

/**
* Returns a BabyJub-compatible value. This function is modified from
* the MACI's genRandomBabyJubValue(). Instead of returning random value
* for the private key, it derives the private key from the users
* signature hash
* Derives the MACI private key from the users signature hash
* @param hash - user's signature hash
* @return The MACI private key
*/
function genPrivKey(hash: string): PrivKey {
// Prevent modulo bias
//const lim = BigInt('0x10000000000000000000000000000000000000000000000000000000000000000')
//const min = (lim - SNARK_FIELD_SIZE) % SNARK_FIELD_SIZE
const min = BigInt(
'6350874878119819312338956282401532410528162663560392320966563075034087161851'
)

if (!isBytesLike(hash)) {
throw new Error(`Hash must be a hex string: ${hash}`)
throw new Error(`genPrivKey() error. Hash must be a hex string: ${hash}`)
}

let hashBN = BigInt(hash)
// don't think we'll enter the for loop below, but, just in case
for (let counter = 1; hashBN < min; counter++) {
const data = concat([toBeArray(hashBN), toBeArray(counter)])
hashBN = BigInt(keccak256(data))
let rawPrivKey = BigInt(hash)
let pubKey: PubKey | null = null

for (let counter = 1; pubKey === null; counter++) {
try {
const privKey = new PrivKey(rawPrivKey)
// this will throw 'Invalid public key' if key is not on the Baby Jubjub elliptic curve
const keypair = new Keypair(privKey)
pubKey = keypair.pubKey
} catch {
const data = concat([toBeArray(rawPrivKey), toBeArray(counter)])
rawPrivKey = BigInt(keccak256(data))
}
}

const rawPrivKey = hashBN % SNARK_FIELD_SIZE
return new PrivKey(rawPrivKey)
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/AnyOldERC20Token.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import '@openzeppelin/contracts/token/ERC20/ERC20.sol';

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/CloneFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

pragma solidity 0.8.10;
pragma solidity 0.8.20;

contract CloneFactory { // implementation of eip-1167 - see https://eips.ethereum.org/EIPS/eip-1167
function createClone(address target) internal returns (address result) {
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/ClrFund.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity 0.8.10;
pragma solidity 0.8.20;

import '@openzeppelin/contracts/token/ERC20/ERC20.sol';
import '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/ClrFundDeployer.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity 0.8.10;
pragma solidity 0.8.20;

import {MACIFactory} from './MACIFactory.sol';
import {ClrFund} from './ClrFund.sol';
Expand All @@ -9,7 +9,7 @@ import {SignUpGatekeeper} from "maci-contracts/contracts/gatekeepers/SignUpGatek
import {InitialVoiceCreditProxy} from "maci-contracts/contracts/initialVoiceCreditProxy/InitialVoiceCreditProxy.sol";
import {Ownable} from '@openzeppelin/contracts/access/Ownable.sol';

contract ClrFundDeployer is CloneFactory, Ownable {
contract ClrFundDeployer is CloneFactory, Ownable(msg.sender) {
address public clrfundTemplate;
address public maciFactory;
address public roundFactory;
Expand Down
3 changes: 1 addition & 2 deletions contracts/contracts/ExternalContacts.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

/*
* These imports are just for hardhat to find the contracts for deployment
Expand All @@ -9,5 +9,4 @@ pragma solidity ^0.8.10;
import {Poll} from 'maci-contracts/contracts/Poll.sol';
import {PollFactory} from 'maci-contracts/contracts/PollFactory.sol';
import {TallyFactory} from 'maci-contracts/contracts/TallyFactory.sol';
import {SubsidyFactory} from 'maci-contracts/contracts/SubsidyFactory.sol';
import {MessageProcessorFactory} from 'maci-contracts/contracts/MessageProcessorFactory.sol';
12 changes: 6 additions & 6 deletions contracts/contracts/FundingRound.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity 0.8.10;
pragma solidity 0.8.20;

import '@openzeppelin/contracts/access/Ownable.sol';
import '@openzeppelin/contracts/token/ERC20/ERC20.sol';
Expand All @@ -15,7 +15,7 @@ import {SignUpGatekeeper} from 'maci-contracts/contracts/gatekeepers/SignUpGatek
import {InitialVoiceCreditProxy} from 'maci-contracts/contracts/initialVoiceCreditProxy/InitialVoiceCreditProxy.sol';
import {CommonUtilities} from 'maci-contracts/contracts/utilities/CommonUtilities.sol';
import {SnarkCommon} from 'maci-contracts/contracts/crypto/SnarkCommon.sol';
import {ITallySubsidyFactory} from 'maci-contracts/contracts/interfaces/ITallySubsidyFactory.sol';
import {ITallyFactory} from 'maci-contracts/contracts/interfaces/ITallyFactory.sol';
import {IMessageProcessorFactory} from 'maci-contracts/contracts/interfaces/IMPFactory.sol';
import {IClrFund} from './interfaces/IClrFund.sol';
import {IMACIFactory} from './interfaces/IMACIFactory.sol';
Expand All @@ -25,7 +25,7 @@ import './userRegistry/IUserRegistry.sol';
import './recipientRegistry/IRecipientRegistry.sol';

contract FundingRound is
Ownable,
Ownable(msg.sender),
SignUpGatekeeper,
InitialVoiceCreditProxy,
DomainObjs,
Expand Down Expand Up @@ -221,10 +221,10 @@ contract FundingRound is
address vkRegistry = address(tally.vkRegistry());

IMessageProcessorFactory messageProcessorFactory = maci.messageProcessorFactory();
ITallySubsidyFactory tallyFactory = maci.tallyFactory();
ITallyFactory tallyFactory = maci.tallyFactory();

address mp = messageProcessorFactory.deploy(verifier, vkRegistry, address(poll), coordinator);
address newTally = tallyFactory.deploy(verifier, vkRegistry, address(poll), mp, coordinator);
address mp = messageProcessorFactory.deploy(verifier, vkRegistry, address(poll), coordinator, Mode.QV);
address newTally = tallyFactory.deploy(verifier, vkRegistry, address(poll), mp, coordinator, Mode.QV);
_setTally(newTally);
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/FundingRoundFactory.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {FundingRound} from './FundingRound.sol';
import {IClrFund} from './interfaces/IClrFund.sol';
Expand Down
5 changes: 1 addition & 4 deletions contracts/contracts/MACICommon.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

/**
* @dev a contract that holds common MACI structures
Expand All @@ -16,9 +16,6 @@ contract MACICommon {
struct Factories {
address pollFactory;
address tallyFactory;
// subsidyFactory is not currently used, it's just a place holder here
address subsidyFactory;
address messageProcessorFactory;
}

}
25 changes: 13 additions & 12 deletions contracts/contracts/MACIFactory.sol
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {MACI} from 'maci-contracts/contracts/MACI.sol';
import {IPollFactory} from 'maci-contracts/contracts/interfaces/IPollFactory.sol';
import {ITallySubsidyFactory} from 'maci-contracts/contracts/interfaces/ITallySubsidyFactory.sol';
import {ITallyFactory} from 'maci-contracts/contracts/interfaces/ITallyFactory.sol';
import {IMessageProcessorFactory} from 'maci-contracts/contracts/interfaces/IMPFactory.sol';
import {SignUpGatekeeper} from 'maci-contracts/contracts/gatekeepers/SignUpGatekeeper.sol';
import {InitialVoiceCreditProxy} from 'maci-contracts/contracts/initialVoiceCreditProxy/InitialVoiceCreditProxy.sol';
Expand All @@ -17,7 +17,7 @@ import {Params} from 'maci-contracts/contracts/utilities/Params.sol';
import {DomainObjs} from 'maci-contracts/contracts/utilities/DomainObjs.sol';
import {MACICommon} from './MACICommon.sol';

contract MACIFactory is Ownable, Params, SnarkCommon, DomainObjs, MACICommon {
contract MACIFactory is Ownable(msg.sender), Params, SnarkCommon, DomainObjs, MACICommon {

// Verifying Key Registry containing circuit parameters
VkRegistry public vkRegistry;
Expand All @@ -41,7 +41,6 @@ contract MACIFactory is Ownable, Params, SnarkCommon, DomainObjs, MACICommon {
error InvalidVkRegistry();
error InvalidPollFactory();
error InvalidTallyFactory();
error InvalidSubsidyFactory();
error InvalidMessageProcessorFactory();
error InvalidVerifier();

Expand Down Expand Up @@ -135,15 +134,17 @@ contract MACIFactory is Ownable, Params, SnarkCommon, DomainObjs, MACICommon {
_stateTreeDepth,
_treeDepths.messageTreeDepth,
_treeDepths.voteOptionTreeDepth,
messageBatchSize)
messageBatchSize,
Mode.QV)
) {
revert ProcessVkNotSet();
}

if (!vkRegistry.hasTallyVk(
_stateTreeDepth,
_treeDepths.intStateTreeDepth,
_treeDepths.voteOptionTreeDepth)
_treeDepths.voteOptionTreeDepth,
Mode.QV)
) {
revert TallyVkNotSet();
}
Expand Down Expand Up @@ -175,24 +176,25 @@ contract MACIFactory is Ownable, Params, SnarkCommon, DomainObjs, MACICommon {
stateTreeDepth,
treeDepths.messageTreeDepth,
treeDepths.voteOptionTreeDepth,
messageBatchSize)
messageBatchSize,
Mode.QV)
) {
revert ProcessVkNotSet();
}

if (!vkRegistry.hasTallyVk(
stateTreeDepth,
treeDepths.intStateTreeDepth,
treeDepths.voteOptionTreeDepth)
treeDepths.voteOptionTreeDepth,
Mode.QV)
) {
revert TallyVkNotSet();
}

_maci = new MACI(
IPollFactory(factories.pollFactory),
IMessageProcessorFactory(factories.messageProcessorFactory),
ITallySubsidyFactory(factories.tallyFactory),
ITallySubsidyFactory(factories.subsidyFactory),
ITallyFactory(factories.tallyFactory),
signUpGatekeeper,
initialVoiceCreditProxy,
TopupCredit(topupCredit),
Expand All @@ -205,8 +207,7 @@ contract MACIFactory is Ownable, Params, SnarkCommon, DomainObjs, MACICommon {
coordinatorPubKey,
address(verifier),
address(vkRegistry),
// pass false to not deploy the subsidy contract
false
Mode.QV
);

// transfer ownership to coordinator to run the tally scripts
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/OwnableUpgradeable.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

// NOTE: had to copy contracts over since OZ uses a higher pragma than we do in the one's they maintain.

Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/TopupToken.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
Expand All @@ -9,7 +9,7 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
* TopupToken is used by MACI Poll contract to validate the topup credits of a user
* In clrfund, this is only used as gateway to pass the topup amount to the Poll contract
*/
contract TopupToken is ERC20, Ownable {
contract TopupToken is ERC20, Ownable(msg.sender) {
constructor() ERC20("TopupCredit", "TopupCredit") {}

function airdrop(uint256 amount) public onlyOwner {
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IClrFund.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity 0.8.10;
pragma solidity 0.8.20;

import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';
import {IUserRegistry} from '../userRegistry/IUserRegistry.sol';
Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IFundingRound.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IFundingRoundFactory.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/interfaces/IMACIFactory.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import {IVkRegistry} from 'maci-contracts/contracts/interfaces/IVkRegistry.sol';
import {IVerifier} from 'maci-contracts/contracts/interfaces/IVerifier.sol';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import './IRecipientRegistry.sol';

Expand Down
2 changes: 1 addition & 1 deletion contracts/contracts/recipientRegistry/IKlerosGTCR.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

/**
* @dev Interface for Kleros Generalized TCR.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

/**
* @dev Interface of the recipient registry.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-3.0

pragma solidity ^0.8.10;
pragma solidity 0.8.20;

import 'solidity-rlp/contracts/RLPReader.sol';

Expand Down
Loading

0 comments on commit 36743e4

Please sign in to comment.