Skip to content

Commit

Permalink
fix: [audit] ZNS-6 - Get a Free Domain with Undefined Price Configura…
Browse files Browse the repository at this point in the history
…tion (#59)
  • Loading branch information
Whytecrowe authored Oct 31, 2023
2 parents a0fe0d7 + e08ccef commit f42f8de
Show file tree
Hide file tree
Showing 14 changed files with 286 additions and 141 deletions.
3 changes: 1 addition & 2 deletions contracts/price/IZNSFixedPricer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@ interface IZNSFixedPricer is IZNSPricer {
struct PriceConfig {
uint256 price;
uint256 feePercentage;
bool isSet;
}

function priceConfigs(bytes32 domainHash) external view returns (uint256 price, uint256 feePercentage);

function initialize(address _accessController, address _registry) external;

function setPrice(bytes32 domainHash, uint256 _price) external;
Expand Down
8 changes: 8 additions & 0 deletions contracts/price/ZNSCurvePricer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
bytes32 parentHash,
string calldata label
) public view override returns (uint256) {
require(
priceConfigs[parentHash].isSet,
"ZNSCurvePricer: parent's price config has not been set properly through IZNSPricer.setPriceConfig()"
);

uint256 length = label.strlen();
// No pricing is set for 0 length domains
if (length == 0) return 0;
Expand Down Expand Up @@ -103,6 +108,8 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
* @dev Validates the value of the `precisionMultiplier` and the whole config in order to avoid price spikes,
* fires `PriceConfigSet` event.
* Only ADMIN can call this function.
* > This function should ALWAYS be used to set the config, since it's the only place where `isSet` is set to true.
* > Use the other individual setters to modify only, since they do not set this variable!
* @param domainHash The domain hash to set the price config for
* @param priceConfig The new price config to set
*/
Expand All @@ -116,6 +123,7 @@ contract ZNSCurvePricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
priceConfigs[domainHash].minPrice = priceConfig.minPrice;
priceConfigs[domainHash].maxLength = priceConfig.maxLength;
priceConfigs[domainHash].feePercentage = priceConfig.feePercentage;
priceConfigs[domainHash].isSet = true;

_validateConfig(domainHash);

Expand Down
9 changes: 8 additions & 1 deletion contracts/price/ZNSFixedPricer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,15 @@ contract ZNSFixedPricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
*/
// solhint-disable-next-line no-unused-vars
function getPrice(bytes32 parentHash, string calldata label) public override view returns (uint256) {
require(
priceConfigs[parentHash].isSet,
"ZNSFixedPricer: parent's price config has not been set properly through IZNSPricer.setPriceConfig()"
);
return priceConfigs[parentHash].price;
}

/**
* @notice Sets the feePercentage for a domain. Only callable by domain owner/operator.
* @notice Sets the feePercentage for a domain. Only callable by domain owner/operator.
* Emits a `FeePercentageSet` event.
* @dev `feePercentage` is set as a part of the `PERCENTAGE_BASIS` of 10,000 where 1% = 100
* @param domainHash The hash of the domain who sets the feePercentage for subdomains
Expand All @@ -63,6 +67,8 @@ contract ZNSFixedPricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
* @notice Setter for `priceConfigs[domainHash]`. Only domain owner/operator can call this function.
* @dev Sets both `PriceConfig.price` and `PriceConfig.feePercentage` in one call, fires `PriceSet`
* and `FeePercentageSet` events.
* > This function should ALWAYS be used to set the config, since it's the only place where `isSet` is set to true.
* > Use the other individual setters to modify only, since they do not set this variable!
* @param domainHash The domain hash to set the price config for
* @param priceConfig The new price config to set
*/
Expand All @@ -72,6 +78,7 @@ contract ZNSFixedPricer is AAccessControlled, ARegistryWired, UUPSUpgradeable, I
) external override {
setPrice(domainHash, priceConfig.price);
setFeePercentage(domainHash, priceConfig.feePercentage);
priceConfigs[domainHash].isSet = true;
}

/**
Expand Down
7 changes: 7 additions & 0 deletions contracts/types/ICurvePriceConfig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,12 @@ interface ICurvePriceConfig {
* See [getRegistrationFee](#getregistrationfee) for the actual fee calc process.
*/
uint256 feePercentage;

/**
* @notice A boolean value signifying the fact that this whole config has been set.
* This value is checked by other contracts to avoid giving out free domains for users who
* haven't yet managed to set their config.
*/
bool isSet;
}
}
224 changes: 112 additions & 112 deletions hardhat.config.ts
Original file line number Diff line number Diff line change
@@ -1,112 +1,112 @@
/* eslint-disable @typescript-eslint/no-var-requires, @typescript-eslint/no-unused-vars */
require("dotenv").config();

import { HardhatUserConfig } from "hardhat/config";
import * as tenderly from "@tenderly/hardhat-tenderly";
import "@nomicfoundation/hardhat-toolbox";
import "@nomiclabs/hardhat-ethers";
import "@nomicfoundation/hardhat-network-helpers";
import "@nomicfoundation/hardhat-chai-matchers";
import "@openzeppelin/hardhat-upgrades";
import "solidity-coverage";
import "solidity-docgen";
import "hardhat-gas-reporter";

// This call is needed to initialize Tenderly with Hardhat,
// the automatic verifications, though, don't seem to work,
// needing us to verify explicitly in code, however,
// for Tenderly to work properly with Hardhat this method
// needs to be called. The call below is commented out
// because if we leave it here, solidity-coverage
// does not work properly locally or in CI, so we
// keep it commented out and uncomment when using DevNet
// locally.
// !!! Uncomment this when using Tenderly DevNet !!!
// tenderly.setup({ automaticVerifications: false });

const config : HardhatUserConfig = {
solidity: {
compilers: [
{
version: "0.8.18",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
],
overrides: {
"@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol": {
version: "0.8.9",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
"@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol": {
version: "0.8.9",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
},
},
paths: {
sources: "./contracts",
tests: "./test",
cache: "./cache",
artifacts: "./artifacts",
},
typechain: {
outDir: "typechain",
},
mocha: {
timeout: 5000000,
},
gasReporter: {
enabled: false
},
networks: {
mainnet: {
url: "https://mainnet.infura.io/v3/97e75e0bbc6a4419a5dd7fe4a518b917",
gasPrice: 80000000000,
},
goerli: {
url: "https://goerli.infura.io/v3/77c3d733140f4c12a77699e24cb30c27",
timeout: 10000000,
},
devnet: {
// Add current URL that you spawned if not using automated spawning
url: `${process.env.DEVNET_RPC_URL}`,
chainId: 1,
},
},
etherscan: {
apiKey: `${process.env.ETHERSCAN_API_KEY}`,
},
tenderly: {
project: `${process.env.TENDERLY_PROJECT_SLUG}`,
username: `${process.env.TENDERLY_ACCOUNT_ID}`,
},
docgen: {
pages: "files",
templates: "docs/docgen-templates",
outputDir: "docs/contracts",
exclude: [
"upgrade-test-mocks/",
"upgradeMocks/",
"token/mocks/",
"utils/",
"oz-proxies/",
],
},
};

export default config;
/* eslint-disable @typescript-eslint/no-var-requires, @typescript-eslint/no-unused-vars */
require("dotenv").config();

import { HardhatUserConfig } from "hardhat/config";
import * as tenderly from "@tenderly/hardhat-tenderly";
import "@nomicfoundation/hardhat-toolbox";
import "@nomiclabs/hardhat-ethers";
import "@nomicfoundation/hardhat-network-helpers";
import "@nomicfoundation/hardhat-chai-matchers";
import "@openzeppelin/hardhat-upgrades";
import "solidity-coverage";
import "solidity-docgen";
import "hardhat-gas-reporter";

// This call is needed to initialize Tenderly with Hardhat,
// the automatic verifications, though, don't seem to work,
// needing us to verify explicitly in code, however,
// for Tenderly to work properly with Hardhat this method
// needs to be called. The call below is commented out
// because if we leave it here, solidity-coverage
// does not work properly locally or in CI, so we
// keep it commented out and uncomment when using DevNet
// locally.
// !!! Uncomment this when using Tenderly DevNet !!!
// tenderly.setup({ automaticVerifications: false });

const config : HardhatUserConfig = {
solidity: {
compilers: [
{
version: "0.8.18",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
],
overrides: {
"@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol": {
version: "0.8.9",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
"@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol": {
version: "0.8.9",
settings: {
optimizer: {
enabled: true,
runs: 200,
},
},
},
},
},
paths: {
sources: "./contracts",
tests: "./test",
cache: "./cache",
artifacts: "./artifacts",
},
typechain: {
outDir: "typechain",
},
mocha: {
timeout: 5000000,
},
gasReporter: {
enabled: false,
},
networks: {
mainnet: {
url: "https://mainnet.infura.io/v3/97e75e0bbc6a4419a5dd7fe4a518b917",
gasPrice: 80000000000,
},
goerli: {
url: "https://goerli.infura.io/v3/77c3d733140f4c12a77699e24cb30c27",
timeout: 10000000,
},
devnet: {
// Add current URL that you spawned if not using automated spawning
url: `${process.env.DEVNET_RPC_URL}`,
chainId: 1,
},
},
etherscan: {
apiKey: `${process.env.ETHERSCAN_API_KEY}`,
},
tenderly: {
project: `${process.env.TENDERLY_PROJECT_SLUG}`,
username: `${process.env.TENDERLY_ACCOUNT_ID}`,
},
docgen: {
pages: "files",
templates: "docs/docgen-templates",
outputDir: "docs/contracts",
exclude: [
"upgrade-test-mocks/",
"upgradeMocks/",
"token/mocks/",
"utils/",
"oz-proxies/",
],
},
};

export default config;
6 changes: 6 additions & 0 deletions test/ZNSCurvePricer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ describe("ZNSCurvePricer", () => {
minPrice: parseEther("10"),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

// as a user of "domainHash" that's not 0x0
Expand Down Expand Up @@ -300,6 +301,7 @@ describe("ZNSCurvePricer", () => {
minPrice: parseEther("6"),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

await expect(
Expand Down Expand Up @@ -331,6 +333,7 @@ describe("ZNSCurvePricer", () => {
minPrice: parseEther("6"),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

await expect(
Expand All @@ -350,6 +353,7 @@ describe("ZNSCurvePricer", () => {
minPrice: parseEther("10"),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

const tx = zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig);
Expand All @@ -373,6 +377,7 @@ describe("ZNSCurvePricer", () => {
minPrice: parseEther("2"),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

const tx = zns.curvePricer.connect(user).setPriceConfig(domainHash, newConfig);
Expand Down Expand Up @@ -633,6 +638,7 @@ describe("ZNSCurvePricer", () => {
minPrice: BigNumber.from(10),
precisionMultiplier: precisionMultiDefault,
feePercentage: registrationFeePercDefault,
isSet: true,
};

// We use `baseLength == 0` to indicate a special event like a promo or discount and always
Expand Down
3 changes: 3 additions & 0 deletions test/ZNSFixedPricer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ describe("ZNSFixedPricer", () => {
{
price: newPrice,
feePercentage: newFee,
isSet: true,
}
);

Expand Down Expand Up @@ -199,6 +200,7 @@ describe("ZNSFixedPricer", () => {
{
price: newPrice,
feePercentage: newFee,
isSet: true,
}
);

Expand All @@ -220,6 +222,7 @@ describe("ZNSFixedPricer", () => {
{
price: BigNumber.from(1),
feePercentage: BigNumber.from(1),
isSet: true,
}
)
).to.be.revertedWith(
Expand Down
1 change: 1 addition & 0 deletions test/ZNSRegistry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
// eslint-disable-next-line @typescript-eslint/no-var-requires
require("@nomicfoundation/hardhat-chai-matchers");


describe("ZNSRegistry", () => {
let deployer : SignerWithAddress;
let operator : SignerWithAddress;
Expand Down
Loading

0 comments on commit f42f8de

Please sign in to comment.