Skip to content

Conversation

@dmbaturin
Copy link
Member

Change summary

Executing commands with root privileges on behalf of operator-level users in a user-supplied environment defeats the purpose because the user can then influence the execution by overriding environment variables used by the system or scripts — in the simplest case, by placing a script named ip in some location and adding that to $PATH.

We need to execute commands in a carefully-prepared environment that includes only knowingly-safe values.

This solution is modeled after doas — a lightweight alternative to sudo that originates from OpenBSD.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Related PR(s)

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

to prevent malicious variable injection
@sever-sever sever-sever requested a review from jestabro November 11, 2025 14:18
Copy link
Contributor

@jestabro jestabro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is well reasoned; a quick sanity check reveals no issue with build and run.

Copy link

@hedrok hedrok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm neither OCaml nor security expert, but everything looks quite robust.
I've launched strace like this:

strace -o trace ./vyos_op_run.exe --debug --dry-run show version

I can't see any dependencies on calls/files that non-priviliged user can exploit.

(had to comment out (*let () = Unix.setuid 0 in*) for strace to work)

@jestabro jestabro merged commit dbecbd0 into vyos:current Nov 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants