Audit finding LIBDEPLOY-04 — dimension 5, severity LOW. Whole-repo audit pass 1 at 440e90b5.
src/lib/LibRainDeploySnapshot.sol:103-125
Problem
isStrictTriple accepts any non-empty run of digits per component, so 0.01.5 is as strict a triple as 0.1.5 and tagForVersion maps it to the directory 0_01_5, which isTag accepts as a release. Two directories then describe one release:
freeze (742-750) refuses a re-cut by vm.exists(dirForSnapshot(tag)). With 0_1_5 already frozen, bumping [package].version to 0.01.5 makes frozenDir a path that does not exist, so the same release is frozen a second time under a second name — the append-only guarantee the error exists for does not fire.recordPrecedes (445-455) reads both tags through vm.parseUint, so 0_01_5 and 0_1_5 compare EQUAL as versions and fall through to the byte-for-byte path tie break, which orders them by the accident of a zero character rather than by release.- both are then declared as separate released suites (
releasedLibraryBlock appends the tag to the key), and both are demanded live on every network by the chain group.
The file states its own rule as "the ONE definition of the release-version shape" whose purpose is that two spellings cannot drift apart — but it admits two spellings of one version, which is the same class of defect one step down. The version is also the soldeer [package].version, where 0.01.5 is not a version any consumer can resolve.
Proposed fix
Refuse a leading zero on a multi-digit component, so a component has exactly one spelling. 0 itself stays legal — 0.0.0 is a real version and is pinned by testIsTagAcceptsWhatAFreezeCanWrite.
In isStrictTriple, replace the digit branch (117-118):
} else if (char >= "0" && char <= "9") {
// A component is one number, so it has one spelling. `01` and
// `1` are the same release and would freeze to two directories,
// neither of which `SnapshotAlreadyFrozen` sees as the other,
// and which `recordPrecedes` cannot order because they compare
// equal as versions.
if (digitsInComponent == 1 && subjectBytes[i - 1] == "0") {
return false;
}
digitsInComponent++;
} else {(i is at least 1 wherever digitsInComponent == 1, because that digit was read at an earlier index.)
Tests (test/src/lib/LibRainDeploySnapshot.t.sol):
/// A component has ONE spelling. A padded component is the same release as
/// its unpadded form: it freezes to a second directory the immutability
/// check does not recognise as the first, and the two compare equal as
/// versions so nothing can order them either.
function testIsStrictTripleRefusesLeadingZeros() external pure {
assertFalse(LibRainDeploySnapshot.isStrictTriple("0.01.5", "."));
assertFalse(LibRainDeploySnapshot.isStrictTriple("01.1.5", "."));
assertFalse(LibRainDeploySnapshot.isStrictTriple("0.1.05", "."));
assertFalse(LibRainDeploySnapshot.isStrictTriple("00.0.0", "."));
assertFalse(LibRainDeploySnapshot.isTag("0_01_5"));
// The boundary: a single zero IS the number zero, and `0.0.0` is a real
// version.
assertTrue(LibRainDeploySnapshot.isStrictTriple("0.0.0", "."));
assertTrue(LibRainDeploySnapshot.isStrictTriple("0.10.0", "."));
assertTrue(LibRainDeploySnapshot.isStrictTriple("10.0.100", "."));
assertTrue(LibRainDeploySnapshot.isTag("0_0_0"));
}
/// The refusal MUST be reachable through the release path, naming the
/// version, rather than only through the predicate.
function testTagForVersionRefusesLeadingZeros() external {
string[3] memory bad = ["0.01.5", "01.1.5", "0.1.05"];
for (uint256 i = 0; i < bad.length; i++) {
vm.expectRevert(abi.encodeWithSelector(UnreleasableVersion.selector, bad[i]));
this.externalTagForVersion(bad[i]);
}
}The existing fuzz testEveryFreezableVersionIsATagTheRecordFinds keeps passing: vm.toString of a uint256 never emits a leading zero.
Verification — this finding survived an adversarial refutation pass
Could not refute; every mechanical claim verified against source and empirically against a forge probe (since removed).
Source read in full: /home/gildlab/code/rain-deploy-audit/src/lib/LibRainDeploySnapshot.sol.
isStrictTriple (103-125) counts digits per component and never inspects the first digit, so 0.01.5 is accepted. isTag is the same predicate with _. tagForVersion (146-158) maps byte-for-byte, so 0.01.5 -> 0_01_5.freeze (742-750) gates a re-cut solely on vm.exists(dirForSnapshot(tag)), and frozenSnapshotPaths (217-250) admits any isTag directory, so 0_01_5 is a second, independently frozen, independently declared release of the same code.recordPrecedes (445-455) reads components with vm.parseUint.
Probe (ran under nix develop -c forge test, PASS, file deleted afterwards): isStrictTriple("0.01.5", ".") true, isTag("0_01_5") true, tagForVersion("0.01.5") == "0_01_5", vm.parseUint("01") == 1, vm.parseUint("00") == 0, and recordPrecedes(vm, "src/generated/0_01_5/A.sol", "src/generated/0_1_5/A.sol") true — i.e. the two tags do compare EQUAL as versions and are ordered by the zero character in the byte-for-byte tiebreak, exactly as described.
Grepped the test tree: test/src/lib/LibRainDeploySnapshot.t.sol covers 0_1_7-rc1, 0_1, 0_1_, _1_7, "", 0.1.7 , 0..7, a.b.c — nothing anywhere asserts anything about a leading zero, and the fuzz testEveryFreezableVersionIsATagTheRecordFinds builds versions with vm.toString(uint256), which never emits one. No guard elsewhere: the only version canonicality check in the repo is this predicate (deployTag -> tagForVersion), and RainDeployVerifySnapshot checks declaration-vs-record, which a duplicate spelling satisfies on both sides. src/generated currently holds only candidate/, so the scenario is prospective, not present.
Not refuted on "correct behaviour" grounds either: the file states its own job as the ONE canonical release-version shape and already refuses cosmetic-only deviations (trailing space, rc suffix), so admitting two spellings of one version is inside its declared responsibility, and recordPrecedes establishes internally that tags are numbers — which is precisely what makes the two directories indistinguishable as releases while remaining distinct as paths.
Severity corrected LOW -> INFO. Value at risk in production is negligible: reaching it requires a maintainer to hand-write a non-semver [package].version (releases are triggered by a hand-typed sol-v* git tag), and the worst outcome is a duplicate frozen directory whose contracts are the same bytecode at the same CREATE2 addresses (so every chain check still passes) plus a Soldeer publish under an unresolvable version, which is loud. No consensus value, no address, and no consumer pin is wrong in any branch — the already-frozen bytes are never replaced, so the immutability the error protects is not itself breached; what is breached is "one release is cut once".
Audit finding
LIBDEPLOY-04— dimension 5, severity LOW. Whole-repo audit pass 1 at440e90b5.src/lib/LibRainDeploySnapshot.sol:103-125Problem
isStrictTripleaccepts any non-empty run of digits per component, so0.01.5is as strict a triple as0.1.5andtagForVersionmaps it to the directory0_01_5, whichisTagaccepts as a release. Two directories then describe one release:freeze(742-750) refuses a re-cut byvm.exists(dirForSnapshot(tag)). With0_1_5already frozen, bumping[package].versionto0.01.5makesfrozenDira path that does not exist, so the same release is frozen a second time under a second name — the append-only guarantee the error exists for does not fire.recordPrecedes(445-455) reads both tags throughvm.parseUint, so0_01_5and0_1_5compare EQUAL as versions and fall through to the byte-for-byte path tie break, which orders them by the accident of a zero character rather than by release.releasedLibraryBlockappends the tag to the key), and both are demanded live on every network by the chain group.The file states its own rule as "the ONE definition of the release-version shape" whose purpose is that two spellings cannot drift apart — but it admits two spellings of one version, which is the same class of defect one step down. The version is also the soldeer
[package].version, where0.01.5is not a version any consumer can resolve.Proposed fix
Refuse a leading zero on a multi-digit component, so a component has exactly one spelling.
0itself stays legal —0.0.0is a real version and is pinned bytestIsTagAcceptsWhatAFreezeCanWrite.In
isStrictTriple, replace the digit branch (117-118):(
iis at least 1 whereverdigitsInComponent == 1, because that digit was read at an earlier index.)Tests (test/src/lib/LibRainDeploySnapshot.t.sol):
The existing fuzz
testEveryFreezableVersionIsATagTheRecordFindskeeps passing:vm.toStringof auint256never emits a leading zero.Verification — this finding survived an adversarial refutation pass
Could not refute; every mechanical claim verified against source and empirically against a forge probe (since removed).
Source read in full: /home/gildlab/code/rain-deploy-audit/src/lib/LibRainDeploySnapshot.sol.
isStrictTriple(103-125) counts digits per component and never inspects the first digit, so0.01.5is accepted.isTagis the same predicate with_.tagForVersion(146-158) maps byte-for-byte, so0.01.5->0_01_5.freeze(742-750) gates a re-cut solely onvm.exists(dirForSnapshot(tag)), andfrozenSnapshotPaths(217-250) admits anyisTagdirectory, so0_01_5is a second, independently frozen, independently declared release of the same code.recordPrecedes(445-455) reads components withvm.parseUint.Probe (ran under
nix develop -c forge test, PASS, file deleted afterwards):isStrictTriple("0.01.5", ".")true,isTag("0_01_5")true,tagForVersion("0.01.5") == "0_01_5",vm.parseUint("01") == 1,vm.parseUint("00") == 0, andrecordPrecedes(vm, "src/generated/0_01_5/A.sol", "src/generated/0_1_5/A.sol")true — i.e. the two tags do compare EQUAL as versions and are ordered by the zero character in the byte-for-byte tiebreak, exactly as described.Grepped the test tree: test/src/lib/LibRainDeploySnapshot.t.sol covers
0_1_7-rc1,0_1,0_1_,_1_7,"",0.1.7,0..7,a.b.c— nothing anywhere asserts anything about a leading zero, and the fuzztestEveryFreezableVersionIsATagTheRecordFindsbuilds versions withvm.toString(uint256), which never emits one. No guard elsewhere: the only version canonicality check in the repo is this predicate (deployTag->tagForVersion), and RainDeployVerifySnapshot checks declaration-vs-record, which a duplicate spelling satisfies on both sides. src/generated currently holds onlycandidate/, so the scenario is prospective, not present.Not refuted on "correct behaviour" grounds either: the file states its own job as the ONE canonical release-version shape and already refuses cosmetic-only deviations (trailing space, rc suffix), so admitting two spellings of one version is inside its declared responsibility, and
recordPrecedesestablishes internally that tags are numbers — which is precisely what makes the two directories indistinguishable as releases while remaining distinct as paths.Severity corrected LOW -> INFO. Value at risk in production is negligible: reaching it requires a maintainer to hand-write a non-semver
[package].version(releases are triggered by a hand-typedsol-v*git tag), and the worst outcome is a duplicate frozen directory whose contracts are the same bytecode at the same CREATE2 addresses (so every chain check still passes) plus a Soldeer publish under an unresolvable version, which is loud. No consensus value, no address, and no consumer pin is wrong in any branch — the already-frozen bytes are never replaced, so the immutability the error protects is not itself breached; what is breached is "one release is cut once".