`PluginRepo` first version is not `1.0`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-aragon/blob/main/packages/contracts/src/framework/plugin/repo/PluginRepo.sol#L165

Vulnerability details

Impact

When the PluginRepo contract is deployed the first version created should be 1.0 but due to an error in the code it will be 1.1 which can impact the protocol working.

Proof of Concept

The first version created of a given PluginRepo should always be equal to 1.0 as it is stated in the docs, but due to an error in the createVersion function the actual first version will be 1.1.

The error occurs in the line below :

File: PluginRepo.sol Line 165

uint16 build = ++buildsPerRelease[_release];

As you can see the code increments first the buildsPerRelease value then assign the final value to the variable build which is used to create the version value as it can be seen :

Tag memory tag = Tag(_release, build);
bytes32 _tagHash = tagHash(tag);

So when the first version is created the variable buildsPerRelease[_release] will be equal to 0 (by default) and thus after the pre-incrementation the value of build will be equal to 1, in consequence the first version created will be 1.1 instead of 1.0.

This can affect the protocol working as the DAO rely on specific versions of a given plugin in its logic and an error in the version can potentially lead to the execution of wrong functionalities or to a bad behaviour of the DAO.

Tools Used

Manual review

Recommended Mitigation Steps

To remove this issue the value of buildsPerRelease[_release] should be incremented after assigning its old value to the build variable, thus the line 165 should be replace by the following :

uint16 build = buildsPerRelease[_release]++;

[0xean]

Believe this to be working as intended, with a version and a build number. Will leave open for sponsor comment to confirm.

[novaknole20]

The documentation is wrong here. It should have stated that for the first version, the release number is 1 and build number is 1 (v1.1). So this can be downgraded to a QA finding that we will acknowledge.

[c4-sponsor]

novaknole20 marked the issue as sponsor disputed

[c4-sponsor]

novaknole20 requested judge review

[c4-sponsor]

novaknole20 marked the issue as disagree with severity

[c4-judge]

0xean changed the severity to QA (Quality Assurance)

[0xean]

Warden fails to demonstrate impact required for this to be considered M and based on sponsor comment, this appears to just be a documentation issue. Downgrading to QA

[c4-judge]

0xean marked the issue as grade-c