Skip to content

Unsafe type assertions in ExtractClientIPStr (common.go) #3206

Description

@hklcf

Bug Description

The function ExtractClientIPStr in common.go uses unsafe type assertions ((*net.UDPAddr) and (*net.TCPAddr)) without the comma-ok idiom. If the clientProto label does not match the actual concrete type behind the net.Addr interface, the function panics.

Affected Code

common.go:182-195:

func ExtractClientIPStr(pluginsState *PluginsState) (string, bool) {
    if pluginsState.clientAddr == nil {
        return "", false
    }
    switch pluginsState.clientProto {
    case "udp":
        return (*pluginsState.clientAddr).(*net.UDPAddr).IP.String(), true  // UNSAFE
    case "tcp", "local_doh":
        return (*pluginsState.clientAddr).(*net.TCPAddr).IP.String(), true  // UNSAFE
    default:
        return "", false
    }
}

This is called from multiple plugin eval paths (block, allow, NX, query log) during query processing.

Impact

A crafted query or internal state inconsistency could cause a type assertion panic, crashing the entire proxy. Note that monitoring_ui.go correctly uses the safe form with comma-ok.

Fix

Use the comma-ok pattern for type assertions:

case "udp":
    if udpAddr, ok := (*pluginsState.clientAddr).(*net.UDPAddr); ok && udpAddr != nil {
        return udpAddr.IP.String(), true
    }
case "tcp", "local_doh":
    if tcpAddr, ok := (*pluginsState.clientAddr).(*net.TCPAddr); ok && tcpAddr != nil {
        return tcpAddr.IP.String(), true
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    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