Skip to content

Use of eval introduces possibility of shell injection unnecessarily. #50

@charles-dyfis-net

Description

@charles-dyfis-net

Unfortunately, fixing this will require changing the configuration format -- it may need to wait for a new major release.

Current configuration is of the form:

options:
- "--exclude '*'"
- "--include 'results/*'"

...that is to say, correct shell escaping is required to be part of the data and the boundary between individual options is not meaningful (for example, --exclude is one argument and * is another, but they're entered as part of the same list element; one could add - "--exclude '*' --include 'results/*'" as a single element with the exact same semantic meaning).

The use of eval makes this possible, but that also introduces potential for human error (any command substitution, redirection, globbing, or other syntax present in the options array will be evaluated by the shell executing the scripts).

A safe alternative would be to instead use configuration of the form:

options:
- "--exclude"
- "*"
- "--include"
- "results/*"

...and have jq be responsible for transforming it into a shell array, as follows:

eval "set -- $(printf '%s\n' "$payload" | jq -r '.source.options // [] | @sh')"

...which runs the command:

set -- '--exclude' '*' '--include' 'results/*'

...allowing "$@" to be expanded to the desired arguments later in the script.

Notes

  • For the reasons for using printf over echo, see https://unix.stackexchange.com/a/65819/3113, or the APPLICATION USAGE and RATIONALE sections of the POSIX standard for echo)
  • Passing eval a single string is strongly preferred over passing it a list of strings. Notably, passing eval multiple strings does not meaningfully pass it distinct arguments: the strings it is passed are concatenated together (with spaces between them) into a single larger string before that single, larger string is passed to the parser. I'm glad to provide some examples of cases where this results in unintended behavior should those be desired.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions