salvorio / salvor-contracts

0 stars 0 forks source link

[AMR-05M] Inexistent Validation of Percentages #7

Open HKskn opened 4 months ago

HKskn commented 4 months ago

AMR-05M: Inexistent Validation of Percentages

Type Severity Location
Input Sanitization AssetManager.sol:L107, L119, L127

Description:

The referenced percentages are ultimately utilized in an AssetManager::_getPortionOfBid call and thus should always be at most 100_00, however, this is not guaranteed in the codebase.

Impact:

It is presently possible to configure percentage values that would cause the code to fail as well as percentage values that could capture the full value of a sale, both of which are undesirable behaviours.

Example:

// Function to set the default royalty percentage.
function setDefaultRoyalty(uint96 _defaultRoyalty) external onlyOwner {
    defaultRoyalty = _defaultRoyalty;
}

Recommendation:

We advise them to be sanitized during their configuration as less-than-or-equal-to their maximum permitted value, 100_00, at minimum and as a best practice to be restricted up to a value defined as their maximum by the Salvor team.

HKskn commented 4 months ago

has been removed and will no longer be used.