[Bug] ModeBase ChangeToMode hand-written decoder accepts missing mandatory NewMode field (same class as #71822)

[Chapoly1305]

Summary

The hand-written ChangeToMode::DecodableType::Decode in src/app/clusters/mode-base-server/mode-base-cluster-objects.cpp silently accepts TLV payloads where the mandatory NewMode field is absent. This affects all 9 Mode-derived clusters.

The root cause is the same structural pattern as #71822 — a decoder loop that treats CHIP_END_OF_TLV as success without tracking which mandatory fields were present — but in hand-written SDK code outside the ZAP template coverage.

Root cause

src/app/clusters/mode-base-server/mode-base-cluster-objects.cpp:38-63:

CHIP_ERROR DecodableType::Decode(TLV::TLVReader & reader)
{
    CHIP_ERROR err = CHIP_NO_ERROR;
    TLV::TLVType outer;
    VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
    ReturnErrorOnFailure(reader.EnterContainer(outer));
    while ((err = reader.Next()) == CHIP_NO_ERROR)
    {
        if (!TLV::IsContextTag(reader.GetTag()))
            continue;
        switch (TLV::TagNumFromTag(reader.GetTag()))
        {
        case to_underlying(Fields::kNewMode):
            ReturnErrorOnFailure(DataModel::Decode(reader, newMode));
            break;                      // ← no presence flag
        default:
            break;
        }
    }

    VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
    ReturnErrorOnFailure(reader.ExitContainer(outer));
    return CHIP_NO_ERROR;               // ← returns SUCCESS even if zero fields seen
}

The loop exits on CHIP_END_OF_TLV without checking whether kNewMode (tag 0) was ever decoded. An empty struct {} is treated identically to a well-formed {NewMode=5}.

This is hand-written code, not ZAP-generated. The template fix for #71822 (clusters-Commands.ipp.zapt and cluster-objects-struct.zapt) does not cover this decoder.

Spec requirement

From the ZCL XML:

<command source="client" code="0x00" name="ChangeToMode" response="ChangeToModeResponse" optional="false">
    <arg name="NewMode" type="int8u" optional="false"/>
</command>

Field	Tag	Type	Conformance
NewMode	0	int8u	M (mandatory)

Scope

This single hand-written decoder serves 9 Mode-derived clusters:

Cluster ID	Cluster Name
0x0049	Oven Mode
0x0051	Laundry Washer Mode
0x0052	Refrigerator And Temperature Controlled Cabinet Mode
0x0054	RVC Run Mode
0x0055	RVC Clean Mode
0x0059	Dishwasher Mode
0x009d	Energy EVSE Mode
0x009e	Water Heater Mode
0x009f	Device Energy Management Mode

Impact

newMode defaults to static_cast<uint8_t>(0) when absent. Mode 0 may be a valid manufacturer-defined mode on some clusters — the handler dispatches with NewMode=0 as if the client explicitly requested it. No InvalidCommand error is returned.

Relationship to #71822

Aspect	#71822	This issue
Affected code	Generated by ZAP templates (848 decoders)	Hand-written mode-base-cluster-objects.cpp (1 decoder, 9 clusters)
Loop mechanism	StructDecodeIterator::Next()	TLVReader::Next()
Error mapping	VerifyOrReturnError(err != EOT, CHIP_NO_ERROR)	Implicit via loop exit on CHIP_END_OF_TLV
Fix approach	Template-level __sawField_* + post-loop check	Same pattern, applied manually

Both have the same structural flaw: the decoder cannot distinguish "saw all required fields then hit EOT" from "hit EOT before seeing any field."

Fix

CHIP_ERROR DecodableType::Decode(TLV::TLVReader & reader)
{
    CHIP_ERROR err = CHIP_NO_ERROR;
    TLV::TLVType outer;
    bool sawNewMode = false;                                 // ← added
    VerifyOrReturnError(TLV::kTLVType_Structure == reader.GetType(), CHIP_ERROR_WRONG_TLV_TYPE);
    ReturnErrorOnFailure(reader.EnterContainer(outer));
    while ((err = reader.Next()) == CHIP_NO_ERROR)
    {
        if (!TLV::IsContextTag(reader.GetTag()))
            continue;
        switch (TLV::TagNumFromTag(reader.GetTag()))
        {
        case to_underlying(Fields::kNewMode):
            ReturnErrorOnFailure(DataModel::Decode(reader, newMode));
            sawNewMode = true;                               // ← added
            break;
        default:
            break;
        }
    }

    VerifyOrReturnError(err == CHIP_END_OF_TLV, err);
    ReturnErrorOnFailure(reader.ExitContainer(outer));

    {
        const char * bypass = getenv("MATTER_BYPASS_MANDATORY_CHECK");
        if ((bypass == nullptr || bypass[0] != '1') && !sawNewMode)
            return CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_IB;
    }

    return CHIP_NO_ERROR;
}

Follows the same pattern as the #71822 template fix: per-field presence flag, checked after loop exit, with MATTER_BYPASS_MANDATORY_CHECK escape hatch for testing.

Broader sweep

A sweep of all hand-written decoders in src/app/clusters/ (outside zzz_generated/) confirmed this is the only one with a command-decoder gap. The other 6 are either no-op decoders for fieldless commands or attribute-value loaders that delegate to generated struct decoders (which already have __sawField_* from the #71822 template fix).