Skip to content

GCU vlanintf_validator builds os.system() from CONFIG_DB VLAN_INTERFACE key - #4764

Open
Bharath-cisco wants to merge 1 commit into
sonic-net:masterfrom
Bharath-cisco:vlanintf_validator
Open

GCU vlanintf_validator builds os.system() from CONFIG_DB VLAN_INTERFACE key#4764
Bharath-cisco wants to merge 1 commit into
sonic-net:masterfrom
Bharath-cisco:vlanintf_validator

Conversation

@Bharath-cisco

Copy link
Copy Markdown

What I did

vlanintf_validator() in the Generic Config Updater flushed ARP/neighbor
entries for deleted VLAN interfaces by formatting the raw
VLAN_INTERFACE|<iface>|<ip> key parts directly into a shell command
string. Both parts come from a CONFIG_DB key, which untrusted local
processes can write, so a crafted key containing shell metacharacters
could execute arbitrary commands as root the next time an administrator
applied a config change touching VLAN_INTERFACE (e.g. config rollback,
config apply-patch). This is a command-injection vulnerability (CWE-78).

How I did it

  • Validate iface against the kernel interface-name pattern
    (^[A-Za-z0-9_.-]{1,15}$) and iface_ip with ipaddress.ip_interface()
    before use; skip (without executing anything) any deleted key that fails
    either check, logging the reason.
  • Run the flush as an argv list —
    subprocess.run(["ip", "neigh", "flush", "dev", iface, iface_ip])
    which never invokes a shell, instead of building a command string from
    untrusted key parts.

How to verify it

  • python3 -m py_compile generic_config_updater/services_validator.py
  • Verified the new checks accept a normal (Vlan100, 10.0.0.1/24) key and
    reject the reported PoC key containing shell metacharacters.
  • config apply-patch deleting a VLAN_INTERFACE entry on a live testbed
    still flushes the neighbor entry for a legitimate interface/IP.

Previous command output (if the output of a command-line utility has changed)

N/A

New command output (if the output of a command-line utility has changed)

N/A

…CE key

Signed-off-by: Bharath <bdhanapa@cisco.com>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

---Powered by SONiC BuildBot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants