Skip to content

[ServiceArea] Inverted predicate in HandleSupportedAreasUpdated removes valid Progress entries, keeps orphans #72947

Description

@Alami-Amine

Summary

In Instance::HandleSupportedAreasUpdated (src/app/clusters/service-area-server/ServiceAreaCluster.cpp), the predicate that selects Progress entries to remove is inverted. When the SupportedAreas list shrinks, the cluster deletes the Progress entries for areas that are still supported and keeps the orphaned entries for areas that were removed — the exact opposite of the intended cleanup.

Details

HandleSupportedAreasUpdated performs three cleanups when SupportedAreas changes. The SelectedAreas block and the Progress block should apply the same rule ("drop the entry if its area is no longer in SupportedAreas"), but the two predicates disagree:

// SelectedAreas cleanup — correct: collects areas NOT in SupportedAreas
while (GetSelectedAreaByIndex(i, areaId))
{
    if (!mStorageDelegate->IsSupportedArea(areaId))   // note the !
    {
        areasToRemoveSpan[areasToRemoveIndex] = areaId;
        areasToRemoveIndex++;
    }
    i++;
}

// Progress cleanup — inverted: collects areas that ARE in SupportedAreas
while (mStorageDelegate->GetProgressElementByIndex(i, tempProgressElement))
{
    if (mStorageDelegate->IsSupportedArea(tempProgressElement.areaID))   // missing !
    {
        progressToRemoveSpan[progressToRemoveIndex] = tempProgressElement.areaID;
        progressToRemoveIndex++;
    }
    i++;
}

The collected progressToRemoveSpan is then passed to RemoveProgressElementRaw, so the Progress block removes the wrong set of entries.

Impact

This is a functional / spec-conformance bug, not a security issue (the mutation is server-driven off SupportedAreas changes, not attacker-reachable, and there is no memory-safety or fabric-boundary concern).

Per the Service Area cluster spec, when entries are removed from SupportedAreas the server must adjust SelectedAreas, CurrentArea, and Progress accordingly, and every Progress entry's areaID must correspond to an entry in SupportedAreas. With this bug:

  • Valid Progress entries (areas still in SupportedAreas) are deleted, losing real progress state.
  • Orphan Progress entries (areas removed from SupportedAreas) persist, violating the per-entry invariant that each Progress areaID is a SupportedAreas entry.

Subscribers to the Progress attribute observe stale/incorrect state after any SupportedAreas shrink.

Suggested fix

Negate the predicate so the Progress block matches the SelectedAreas block:

-        if (mStorageDelegate->IsSupportedArea(tempProgressElement.areaID))
+        if (!mStorageDelegate->IsSupportedArea(tempProgressElement.areaID))

A regression test covering a SupportedAreas shrink (asserting still-supported areas keep their Progress entries and removed areas lose theirs) would prevent this from regressing, and a sibling test on the SelectedAreas cleanup would lock in the correct shape.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions