fix(mqtt): SparkplugCodec's never-throw guard must span the projection too
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
This commit is contained in:
@@ -74,36 +74,38 @@ public static class SparkplugCodec
|
|||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
Payload proto;
|
|
||||||
try
|
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)
|
catch (Exception)
|
||||||
{
|
{
|
||||||
// Deliberately broad. InvalidProtocolBufferException covers malformed, truncated and
|
// Deliberately broad, and deliberately spanning the projection as well as the parse.
|
||||||
// over-nested input, but this is the dispatcher-thread boundary: the contract is a
|
// InvalidProtocolBufferException covers malformed, truncated and over-nested input, but
|
||||||
// verdict for EVERY input, and narrowing the catch would trade that guarantee for a
|
// this is the dispatcher-thread boundary and the projection walks an attacker-shaped
|
||||||
// taxonomy nobody downstream can act on.
|
// 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;
|
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;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
|
|||||||
Reference in New Issue
Block a user