Skip to content

DISCONNECT User Property Maximum Packet Size Gate Is Missing on the Generic Send Path #3635

@LiD0209

Description

@LiD0209

DISCONNECT User Property Maximum Packet Size Gate Is Missing on the Generic Send Path

Conclusion

Mosquitto is partially compliant for item 763.

MQTT 5.0 requires a sender not to include DISCONNECT properties if doing so
would make the packet exceed the peer's Maximum Packet Size. Mosquitto has a
shared oversize-check helper, and several packet-building paths do use it.
However, the generic send__disconnect() path builds a DISCONNECT packet
without first calling that helper.

That means the implementation contains the correct infrastructure, but the
generic DISCONNECT send path does not consistently apply it.

MQTT Standard Requirement

  • Standard: MQTT 5.0
  • Section: 3.14.2.2 DISCONNECT Properties

Normative text:

The sender MUST NOT send this property if it would increase the size of the
DISCONNECT packet beyond the Maximum Packet Size specified by the receiver.

That means:

  • the rule is sender-side
  • it applies specifically to optional DISCONNECT properties
  • the packet must be checked before it is sent, not only after the packet
    structure already exists in memory

Relevant Code

The Generic DISCONNECT Path Computes Length but Does Not Check Oversize

lib/send_disconnect.c:

int send__disconnect(struct mosquitto *mosq, uint8_t reason_code, const mosquitto_property *properties)
{
	struct mosquitto__packet *packet = NULL;
	int rc;
	uint32_t remaining_length = 0;
...
	if(mosq->protocol == mosq_p_mqtt5 && (reason_code != 0 || properties)){
		remaining_length = 1;
		if(properties){
			remaining_length += mosquitto_property_get_remaining_length(properties);
		}
	}else{
		remaining_length = 0;
	}

	rc = packet__alloc(&packet, CMD_DISCONNECT, remaining_length);
	if(rc){
		mosquitto_FREE(packet);
		return rc;
	}
	if(remaining_length > 0){
		packet__write_byte(packet, reason_code);
		if(properties){
			property__write_all(packet, properties, true);
		}
	}

This code proves:

  • the function knows exactly how large the MQTT 5 DISCONNECT packet will be
  • it includes property length in remaining_length
  • it allocates and writes the packet directly afterward

What is missing is a call to packet__check_oversize() between length
calculation and packet allocation.

The Shared Oversize Helper Exists

lib/packet_mosq.c:

int packet__check_oversize(struct mosquitto *mosq, uint32_t remaining_length)
{
	uint32_t len;

	if(mosq->maximum_packet_size == 0){
		return MOSQ_ERR_SUCCESS;
	}

	len = 1 + remaining_length + mosquitto_varint_bytes(remaining_length);
	if(len > mosq->maximum_packet_size){
		return MOSQ_ERR_OVERSIZE_PACKET;
	}
	return MOSQ_ERR_SUCCESS;
}

That means the project already has the exact helper needed to enforce the MQTT
5 rule.

Other Packet Paths Do Use the Helper

For comparison, src/send_connack.c performs the check before allocation:

remaining_length += mosquitto_property_get_remaining_length(connack_props);
}

if(packet__check_oversize(context, remaining_length)){
	mosquitto_property_free_all(&connack_props);
	return MOSQ_ERR_OVERSIZE_PACKET;
}

rc = packet__alloc(&packet, CMD_CONNACK, remaining_length);

lib/actions_subscribe.c does the same:

if(mosq->maximum_packet_size > 0){
	remaining_length += 2 + mosquitto_property_get_length_all(outgoing_properties);
	if(packet__check_oversize(mosq, remaining_length)){
		return MOSQ_ERR_OVERSIZE_PACKET;
	}
}

That means item 763 is not a missing-feature problem for the whole codebase.
It is a missing-use-of-existing-check problem on the generic DISCONNECT path.

Why This Is Partial

Implemented:

  • Mosquitto tracks the peer's maximum_packet_size
  • Mosquitto has a shared oversize-check helper
  • several send paths already enforce the packet-size limit correctly

Missing:

  • the generic send__disconnect() path does not call the helper before sending
  • therefore DISCONNECT properties are not uniformly gated by the peer's
    advertised size limit on this path

This is partial rather than unsatisfied because the common implementation
infrastructure exists and many ordinary paths are already safe. The gap is that
one reusable DISCONNECT construction path is still incomplete.

Runtime Evidence

No dedicated runtime reproducer was found in this directory for item 763.

The conclusion is therefore:

source-confirmed partial

That means the standards mismatch is established directly from the packet
construction code.

Fix Direction

A precise fix would be to add the same guard used by other MQTT 5 send paths:

if(packet__check_oversize(mosq, remaining_length)){
	return MOSQ_ERR_OVERSIZE_PACKET;
}

The correct insertion point is after remaining_length is finalized and before
packet__alloc() is called in lib/send_disconnect.c.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Status: AvailableNo one has claimed responsibility for resolving this issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions