Fractional v2 contest - aysha's results

Title: Insufficient Input Validation

Impact

Proof of Concept

Check if “_id” exist or not Check if “_amount” is greater than zero https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L56

Check if “_id” exist or not Check if “_amount” is greater than zero https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L79

Check _operator not to be zero Check if “_id” exist or not https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L186

Check _salePrice to be greater than zero https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L241

Tools Used

Manual

Recommended Mitigation Steps

add input validation specifically to external functions

================================================================== Title: Two-step change of privileged roles

Impact

In a single-step change, if the current admin accidentally changes the new admin to a zero-address or an incorrect address (where the private keys are not available), the system is left without an operational admin and will have to be redeployed.

Proof of Concept

https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L229 https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L93

Tools Used

Manual

Recommended Mitigation Steps

When privileged roles are being changed, it is recommended to follow a two-step approach: 1) The current privileged role proposes a new address for the change 2) The newly proposed address then claims the privileged role in a separate transaction. This two-step change allows accidental proposals to be corrected instead of leaving the system operationally with no/malicious privileged role.

Title: missing check for transfer’s return value

Impact

Should return a boolean. Several tokens do not return a boolean on these functions. As a result, their calls in the contract might fail.

Proof of Concept

See here https://github.com/crytic/building-secure-contracts/blob/master/development-guidelines/token_integration.md#erc-conformity code https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/references/TransferReference.sol#L22

Tools Used

Manual

Recommended Mitigation Steps

Use require

Title: Missing zero address check

Impact

The functions should first check the address.

Proof of Concept

Check _receiver https://github.com/code-423n4/2022-07-fractional/blob/e2c5a962a94106f9495eb96769d7f60f7d5b14c9/src/FERC1155.sol#L217 Check _to https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L311 Check _to https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L343 Check _to https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L376 Check _to https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/modules/Buyout.sol#L413 Check _newOwner https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/Vault.sol#L93 Check _deployer https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/VaultFactory.sol#L33 Check _deployer https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/VaultFactory.sol#L55 Check _owner https://github.com/code-423n4/2022-07-fractional/blob/8f2697ae727c60c93ea47276f8fa128369abfe51/src/VaultFactory.sol#L62

Tools Used

Manual

Recommended Mitigation Steps

Check zero address