java-topology/docs/tickets/mattermost-0001-check-roles-exist-nested-linear-scan.md

1.4 KiB
Raw Permalink Blame History

mattermost-0001: CheckRolesExist() O(n×m) nested loop — linear scan of roles slice per role name

Target: mattermost/mattermost Severity: LOW CWE: CWE-407 (Inefficient Algorithmic Complexity) File: server/channels/app/role.go Lines: 258278 Status: PATCHED

Description

CheckRolesExist() fetches all roles by name (GetRolesByNames) and then performs a nested loop to verify each requested name exists in the result:

for _, name := range roleNames {           // O(n) outer
    nameFound := false
    for _, role := range roles {           // O(m) inner scan
        if name == role.Name {
            nameFound = true
            break
        }
    }
    ...
}

Total work: O(n × m) where n = len(roleNames) and m = len(roles).

This function is called from UpdateUserRoles on user role assignment (user.go:1944), which can be triggered at login or privilege change. While the number of roles is typically small (<50), the pattern is categorically incorrect and sets a bad example.

Fix

Build a set from the returned roles first:

roleSet := make(map[string]bool, len(roles))
for _, role := range roles {
    roleSet[role.Name] = true
}
for _, name := range roleNames {
    if !roleSet[name] {
        return model.NewAppError(...)
    }
}

Complexity

Before After
CheckRolesExist O(n × m) O(n + m)