From c164ec3da112404181c7eb01e033728cb835c327 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 24 Jul 2026 21:10:20 -0400 Subject: [PATCH] fix(mqtt): SparkplugCodec's never-throw guard must span the projection too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The try/catch stopped at Payload.Parser.ParseFrom, leaving the metric-projection loop outside it — so the "never throws for any input" contract had a hole in exactly the part that walks an attacker-shaped object graph. Widened to cover parse AND projection. Self-review finding; no test reddened, because a projection escape needs a generated-code shape the vectors do not produce. That is the point: the guard is the contract, not the observed behaviour of today's inputs. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- .../Sparkplug/SparkplugCodec.cs | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs index 5234cbbe..b428ee19 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugCodec.cs @@ -74,36 +74,38 @@ public static class SparkplugCodec return false; } - Payload proto; try { - proto = Payload.Parser.ParseFrom(wire); + var proto = Payload.Parser.ParseFrom(wire); + + var metrics = proto.Metrics.Count == 0 + ? [] + : new SparkplugMetric[proto.Metrics.Count]; + + for (var i = 0; i < proto.Metrics.Count; i++) + { + metrics[i] = ProjectMetric(proto.Metrics[i]); + } + + payload = new SparkplugPayload( + IsValid: true, + Seq: proto.HasSeq ? proto.Seq : null, + TimestampMs: proto.HasTimestamp ? proto.Timestamp : null, + Metrics: metrics); + + return true; } catch (Exception) { - // Deliberately broad. InvalidProtocolBufferException covers malformed, truncated and - // over-nested input, but this is the dispatcher-thread boundary: the contract is a - // verdict for EVERY input, and narrowing the catch would trade that guarantee for a - // taxonomy nobody downstream can act on. + // Deliberately broad, and deliberately spanning the projection as well as the parse. + // InvalidProtocolBufferException covers malformed, truncated and over-nested input, but + // this is the dispatcher-thread boundary and the projection walks an attacker-shaped + // object graph: the contract is a verdict for EVERY input, so a guard that stopped at + // the parse would be a contract with a hole in it. Narrowing the catch would trade the + // guarantee for a taxonomy nobody downstream can act on. + payload = SparkplugPayload.Invalid; return false; } - - var metrics = proto.Metrics.Count == 0 - ? [] - : new SparkplugMetric[proto.Metrics.Count]; - - for (var i = 0; i < proto.Metrics.Count; i++) - { - metrics[i] = ProjectMetric(proto.Metrics[i]); - } - - payload = new SparkplugPayload( - IsValid: true, - Seq: proto.HasSeq ? proto.Seq : null, - TimestampMs: proto.HasTimestamp ? proto.Timestamp : null, - Metrics: metrics); - - return true; } ///