Strip inner-message padding when sizing unsigned broadcasts (#11083)
This commit is contained in:
co-authored by
GitHub
parent
023351a979
commit
405a6bfd8a
+60
-3
@@ -443,6 +443,62 @@ void Router::sniffReceived(const meshtastic_MeshPacket *p, const meshtastic_Rout
|
||||
}
|
||||
|
||||
#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA)
|
||||
/** Size a decoded Data as the sender's signedDataFits() gate would have, with padding stripped:
|
||||
* unknown fields inside Data.payload survive in payload.size and would otherwise let a forger
|
||||
* inflate an unsigned broadcast past the signable budget. Returns false only if sizing failed.
|
||||
* Sizing only what this build's schema decodes, so a signable type that later grows needs its
|
||||
* legitimate maximum re-checked against the budget or honest unsigned broadcasts get dropped. */
|
||||
static bool canonicalSignableSize(meshtastic_Data *d, size_t *size)
|
||||
{
|
||||
const pb_msgdesc_t *fields = nullptr;
|
||||
switch (d->portnum) {
|
||||
case meshtastic_PortNum_POSITION_APP:
|
||||
fields = &meshtastic_Position_msg;
|
||||
break;
|
||||
case meshtastic_PortNum_TELEMETRY_APP:
|
||||
fields = &meshtastic_Telemetry_msg;
|
||||
break;
|
||||
case meshtastic_PortNum_WAYPOINT_APP:
|
||||
fields = &meshtastic_Waypoint_msg;
|
||||
break;
|
||||
case meshtastic_PortNum_NODEINFO_APP:
|
||||
fields = &meshtastic_User_msg;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
|
||||
if (fields) {
|
||||
// Scratch kept off the stack: these decoded structs are large for the smaller MCU targets.
|
||||
// Safe as file-static state because both callers of checkXeddsaReceivePolicy hold cryptLock.
|
||||
static union {
|
||||
// cppcheck-suppress unusedStructMember ; written by pb_decode through &inner
|
||||
meshtastic_Position position;
|
||||
// cppcheck-suppress unusedStructMember ; written by pb_decode through &inner
|
||||
meshtastic_Telemetry telemetry;
|
||||
// cppcheck-suppress unusedStructMember ; written by pb_decode through &inner
|
||||
meshtastic_Waypoint waypoint;
|
||||
// cppcheck-suppress unusedStructMember ; written by pb_decode through &inner
|
||||
meshtastic_User user;
|
||||
} inner;
|
||||
|
||||
memset(&inner, 0, sizeof(inner));
|
||||
size_t canonicalPayload;
|
||||
if (pb_decode_from_bytes(d->payload.bytes, d->payload.size, fields, &inner) &&
|
||||
pb_get_encoded_size(&canonicalPayload, fields, &inner) && canonicalPayload <= d->payload.size) {
|
||||
// Only the length matters when sizing a bytes field, so swap it in place instead of
|
||||
// copying the whole Data; restored below because modules still need the real payload.
|
||||
const pb_size_t prevSize = d->payload.size;
|
||||
d->payload.size = (pb_size_t)canonicalPayload;
|
||||
const bool sized = pb_get_encoded_size(size, &meshtastic_Data_msg, d);
|
||||
d->payload.size = prevSize;
|
||||
return sized;
|
||||
}
|
||||
}
|
||||
|
||||
return pb_get_encoded_size(size, &meshtastic_Data_msg, d);
|
||||
}
|
||||
|
||||
bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p)
|
||||
{
|
||||
// Only a signature we verify below may mark this packet signed; never trust an inbound flag.
|
||||
@@ -477,12 +533,13 @@ bool checkXeddsaReceivePolicy(meshtastic_MeshPacket *p)
|
||||
// non-PKI broadcast whose signed encoding would still fit the LoRa frame. Size p->decoded
|
||||
// canonically so this counts the same fields the sender's signedDataFits() gate counted;
|
||||
// adding XEDDSA_SIGNATURE_FIELD_BYTES to that unsigned base mirrors it exactly, whatever
|
||||
// fields the Data carried. Unicast/PKI packets and broadcasts too big to carry a signature
|
||||
// are never signed, so they must not be hard-failed here even for a known signer.
|
||||
// fields the Data carried, with padding hidden inside Data.payload stripped. Unicast/PKI
|
||||
// packets and broadcasts too big to carry a signature are never signed, so they must not be
|
||||
// hard-failed here even for a known signer.
|
||||
const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(p->from);
|
||||
if (node && nodeInfoLiteHasXeddsaSigned(node) && !p->pki_encrypted && isBroadcast(p->to)) {
|
||||
size_t canonicalSize;
|
||||
if (!pb_get_encoded_size(&canonicalSize, &meshtastic_Data_msg, &p->decoded))
|
||||
if (!canonicalSignableSize(&p->decoded, &canonicalSize))
|
||||
return true; // can't size it; never drop on a sizing failure
|
||||
if (canonicalSize + XEDDSA_SIGNATURE_FIELD_BYTES + MESHTASTIC_HEADER_LENGTH <= MAX_LORA_PAYLOAD_LEN) {
|
||||
LOG_WARN("Dropping unsigned broadcast from 0x%08x that previously signed", p->from);
|
||||
|
||||
Reference in New Issue
Block a user