Page MenuHomeVyOS Platform

is_intf_addr_assigned() has no input validation and builds its netns command as a shell string
Open, Requires assessmentPublicBUG

Description

Component: vyos-1x (Python library / vyos.utils.network)

Type: Bug


Description

is_intf_addr_assigned() in python/vyos/utils/network.py (current rolling,
lines 487-513) is an internal validation helper called from ~30 places. It has
several defects that are not reachable from the CLI today, because every
in-tree caller passes a value the CLI validators have already constrained. They
should still be fixed before a future caller does hit them.

This is the follow-up agreed with @c-po in
https://github.com/vyos/vyos-1x/pull/5400 — that PR only patched the range
symptom and is being closed in favour of this task.

1. No input validation, ValueError escapes to the caller

>>> is_intf_addr_assigned('lo', '192.0.2.1-192.0.2.2')
ValueError: '192.0.2.1-192.0.2.2' does not appear to be an IPv4 or IPv6 interface

ip_interface(addr) on line 510 raises for anything that is not a single
address. The docstring already states the contract ("a single IP address"), so
the function should enforce it instead of letting an ipaddress exception
surface.

2. Zone suffix is stripped but never compared against ifname

Lines 507-508 remove %zone from the address. Since is_addr_assigned()
loops over every interface and calls this function once per interface, the zone
is silently discarded and never checked against the interface being examined:

>>> is_addr_assigned('::1%eno1')
True          # ::1 is on lo, eno1 never enters the comparison

3. Range plus zone suffix is reported as assigned

The % split runs before any range check, so the range is truncated to its
first element and then matches:

>>> is_intf_addr_assigned('lo', '::1%lo-::2%lo')
True

4. The addr parameter is reassigned inside the loop

Line 508 writes back into the addr parameter on every iteration, so the
stripping is order-dependent on the address list rather than done once up
front.

5. The netns command is built as a shell string

netns_cmd = f'ip netns exec {netns}' if netns else ''
rc, out = rc_cmd(f'{netns_cmd} ip --json address show dev {ifname}')

popen() auto-enables a shell when the command string looks like it needs one,
so ifname and netns are interpolated into a shell command line. Both
rc_cmd() and popen() already accept a netns= argument, which makes the
prefix unnecessary.

6. Dead field in the jmespath projection

Line 502 selects family into the projection; it is never read.

Proposed fix (per @c-po's guidance in #5400)

  • Require addr to be a single IPv4/IPv6 address, tested with vyos.template.is_ip(), and raise an error if it is not. This removes the need to handle % internally at all, and resolves items 1, 2, 3 and 4 in one step — no zone-matching semantics get introduced, invalid input is simply rejected.
  • Pass the namespace through rc_cmd(..., netns=netns) instead of prefixing the command string (item 5).
  • Drop the unused family key (item 6).

All of this is unit-testable in src/tests/test_utils_network.py; no smoketest
is required.

Note on overlap with T8977 / PR #5266

PR #5266 (T8977) touches the same lines and adds

if not is_ipv4_address(addr) and not is_ipv6_address(addr):
    return False

i.e. it rejects invalid input by returning False silently, whereas the
approach agreed here is to raise. These two are mutually exclusive on the
same code path, so the two changes need sequencing or reconciling — #5266 is
still open with CHANGES_REQUESTED as of 2026-09-15.

Details

Version
1.5.1
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Bug (incorrect behavior)