Skip to content

Commit 46eddb3

Browse files
mikeraclaude
andcommitted
harden(dlfs): fail-safe lattice merge against malformed foreign nodes
DLFSLattice.merge called DLFSNode.merge directly on the both-non-null path, so a malformed node from an untrusted peer (wrong-typed POS_UTIME/POS_DIR, too-short vector, or a malformed child) could throw and crash the merge (DoS) on the network propagation path — checkForeign was only enforced when own was null. Wrap the recursive merge so it fails closed to own on any RuntimeException, rather than pre-validating (which would be expensive on deep trees). A malformed value from an untrusted peer is thus ignored — it can neither crash nor corrupt the merged state. Both merge overloads share the safeMerge path. Test: DLFSTombstoneTest.testMergeFailsSafeOnMalformedNode feeds too-short, bad-utime, non-Index-dir and deep-malformed-child nodes through both overloads and asserts the merge keeps own without throwing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent d3cbaac commit 46eddb3

2 files changed

Lines changed: 53 additions & 20 deletions

File tree

convex-core/src/main/java/convex/lattice/fs/DLFSLattice.java

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import convex.core.data.prim.CVMLong;
99
import convex.core.util.Utils;
1010
import convex.lattice.ALattice;
11+
import convex.lattice.LatticeContext;
1112
import convex.lattice.generic.IndexLattice;
1213

1314
/**
@@ -46,10 +47,7 @@ private DLFSLattice() {
4647
public AVector<ACell> merge(AVector<ACell> ownValue, AVector<ACell> otherValue) {
4748
// Handle null cases
4849
if (ownValue == null) {
49-
if (checkForeign(otherValue)) {
50-
return otherValue;
51-
}
52-
return zero();
50+
return checkForeign(otherValue) ? otherValue : zero();
5351
}
5452
if (otherValue == null) {
5553
return ownValue;
@@ -60,34 +58,39 @@ public AVector<ACell> merge(AVector<ACell> ownValue, AVector<ACell> otherValue)
6058
return ownValue;
6159
}
6260

63-
// Delegate to DLFSNode.merge which implements the rsync-like merge logic
64-
// The merge is deterministic: timestamp is derived from the input nodes
65-
return DLFSNode.merge(ownValue, otherValue);
61+
return safeMerge(ownValue, otherValue);
6662
}
6763

6864
@Override
69-
public AVector<ACell> merge(convex.lattice.LatticeContext context, AVector<ACell> ownValue, AVector<ACell> otherValue) {
70-
// Handle null cases
65+
public AVector<ACell> merge(LatticeContext context, AVector<ACell> ownValue, AVector<ACell> otherValue) {
66+
// Context timestamp is not used for DLFS merge — the merge is deterministic from
67+
// the input nodes — so this behaves identically to the no-context overload.
7168
if (ownValue == null) {
72-
if (checkForeign(otherValue)) {
73-
return otherValue;
74-
}
75-
return zero();
69+
return checkForeign(otherValue) ? otherValue : zero();
7670
}
7771
if (otherValue == null) {
7872
return ownValue;
7973
}
80-
81-
// Fast path: if values are equal, return own value
8274
if (Utils.equals(ownValue, otherValue)) {
8375
return ownValue;
8476
}
77+
return safeMerge(ownValue, otherValue);
78+
}
8579

86-
// Delegate to DLFSNode.merge which implements the rsync-like merge logic
87-
// The merge is deterministic: timestamp is derived from the input nodes
88-
// Note: Context timestamp is currently not used for DLFS merge.
89-
// If timestamp override is needed, it should be handled at a higher level.
90-
return DLFSNode.merge(ownValue, otherValue);
80+
/**
81+
* Fail-safe merge of two non-null, unequal nodes. {@code other} may originate from an
82+
* untrusted peer; rather than pre-validating its structure, the merge is attempted and
83+
* falls closed to {@code own} if a malformed node makes it throw. A malformed value can
84+
* therefore neither crash the merge (DoS) nor corrupt the merged state — it is ignored.
85+
*/
86+
private AVector<ACell> safeMerge(AVector<ACell> own, AVector<ACell> other) {
87+
try {
88+
return DLFSNode.merge(own, other);
89+
} catch (RuntimeException e) {
90+
// Malformed / adversarial foreign node: fail closed and keep own, rather than
91+
// letting a bad value from an untrusted peer crash or corrupt the merge.
92+
return own;
93+
}
9194
}
9295

9396
@Override

convex-core/src/test/java/convex/lattice/fs/DLFSTombstoneTest.java

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import static org.junit.jupiter.api.Assertions.assertEquals;
44
import static org.junit.jupiter.api.Assertions.assertFalse;
5+
import static org.junit.jupiter.api.Assertions.assertSame;
56
import static org.junit.jupiter.api.Assertions.assertTrue;
67

78
import java.io.IOException;
89
import java.nio.file.Files;
910
import java.nio.file.Path;
11+
import java.util.List;
1012

1113
import org.junit.jupiter.api.Test;
1214

@@ -15,7 +17,9 @@
1517
import convex.core.data.AVector;
1618
import convex.core.data.Index;
1719
import convex.core.data.Strings;
20+
import convex.core.data.Vectors;
1821
import convex.core.data.prim.CVMLong;
22+
import convex.lattice.LatticeContext;
1923
import convex.lattice.fs.impl.DLFSLocal;
2024

2125
/**
@@ -133,4 +137,30 @@ public void testMergeConverges() throws IOException {
133137
assertTrue(Files.exists(a.getPath("/d/keep")));
134138
assertTrue(Files.exists(a.getPath("/d/added")));
135139
}
140+
141+
/**
142+
* A malformed foreign node from an untrusted peer must not crash or corrupt the merge:
143+
* the merge fails closed to {@code own} rather than throwing.
144+
*/
145+
@Test
146+
public void testMergeFailsSafeOnMalformedNode() {
147+
DLFSLattice lat = DLFSLattice.INSTANCE;
148+
149+
// A valid own directory with one live child
150+
Index<AString, AVector<ACell>> ownEntries =
151+
Index.of(Strings.create("f"), DLFSNode.createEmptyFile(CVMLong.create(1)));
152+
AVector<ACell> own = Vectors.of(ownEntries, null, null, CVMLong.create(1));
153+
154+
AVector<ACell> tooShort = Vectors.of(CVMLong.ZERO); // missing slots
155+
AVector<ACell> badUtime = Vectors.of(Index.none(), null, null, Strings.create("nope")); // POS_UTIME not a long
156+
AVector<ACell> badDir = Vectors.of(Strings.create("x"), null, null, CVMLong.create(2)); // POS_DIR not an Index
157+
Index<AString, AVector<ACell>> badEntries =
158+
Index.of(Strings.create("f"), Vectors.of(Strings.create("x"))); // malformed child node
159+
AVector<ACell> deepBad = Vectors.of(badEntries, null, null, CVMLong.create(2));
160+
161+
for (AVector<ACell> bad : List.of(tooShort, badUtime, badDir, deepBad)) {
162+
assertSame(own, lat.merge(own, bad), "malformed foreign node must be rejected, keeping own");
163+
assertSame(own, lat.merge((LatticeContext) null, own, bad), "context merge must also fail safe");
164+
}
165+
}
136166
}

0 commit comments

Comments
 (0)