Security: Cross-network external client modification via unscoped lookup in updateExtClient

[lighthousekeeper1212]

Summary

The updateExtClient handler in controllers/ext_client.go uses logic.GetExtClientByName(clientid) which searches ALL external clients across ALL networks, while every other ext_client endpoint correctly uses logic.GetExtClient(clientid, network) which scopes the lookup by network. This allows a user with access to one network to modify external clients belonging to a different network.

Vulnerable Code

In controllers/ext_client.go line 921:

func updateExtClient(w http.ResponseWriter, r *http.Request) {
    var params = mux.Vars(r)
    clientid := params["clientid"]
    // BUG: Uses GetExtClientByName which searches ALL networks
    oldExtClient, err := logic.GetExtClientByName(clientid)

GetExtClientByName in logic/clients.go (line 69) iterates ALL clients without network filtering:

func GetExtClientByName(ID string) (models.ExtClient, error) {
    clients, err := GetAllExtClients()  // All clients, all networks
    for i := range clients {
        if clients[i].ClientID == ID {
            return clients[i], nil
        }
    }
}

Secure Comparison

Every other endpoint correctly uses the network-scoped lookup:

Endpoint	Line	Lookup
GET (getExtClient)	131	GetExtClient(clientid, network) ✅
GET conf (getExtClientConf)	172	GetExtClient(clientid, networkid) ✅
POST (createExtClient)	499	GetExtClient(extclient.ClientID, networkid) ✅
PUT (updateExtClient)	921	GetExtClientByName(clientid) ❌
DELETE (deleteExtClient)	1061	GetExtClient(clientid, network) ✅

Note: The {network} URL parameter IS available (line 948: gateway.NetID = params["network"]) but is not used for the initial client lookup.

Impact

• Severity: High - Cross-network boundary violation
• A user with access to Network A can modify external clients (VPN peers) belonging to Network B
• Can change client configuration including allowed IPs, DNS settings, public keys, and ACLs
• Affects network segmentation which is a core security feature

Suggested Fix

Replace line 921:

// Before (vulnerable):
oldExtClient, err := logic.GetExtClientByName(clientid)

// After (fixed):
network := params["network"]
oldExtClient, err := logic.GetExtClient(clientid, network)

Note

Per your SECURITY.md, vulnerabilities should be reported to info@netmaker.io. Since private vulnerability reporting is not enabled on GitHub, I'm filing this as an issue. Please feel free to convert this to a security advisory. I'm happy to coordinate on disclosure timeline.

Discovery

Found through automated security research comparing lookup function usage patterns across ext_client endpoints.