3

I have a situation where a Bash script runs and parses a user-supplied JSON file using jq. Since it's supplied by the user, it's possible for them to include values in the JSON to perform an injection attack.

I'd like to know if there's a way to overcome this. Please note, the setup of: 'my script parsing a user-supplied JSON file' cannot be changed, as it's out of my control. Only thing I can control is the Bash script.

I've tried using jq with and without the -r flag, but in each case, I was successfully able to inject.

Here's what the Bash script looks like at the moment:

#!/bin/bash

set -e

eval "INCLUDES=($(cat user-supplied.json | jq '.Include[]'))"
CMD="echo Includes are: "
for e in "${INCLUDES[@]}"; do
    CMD="$CMD\\\"$e\\\" "
done

eval "$CMD"

And here is an example of a sample user-supplied.json file that demonstrates an injection attack:

{
  "Include": [
    "\\\";ls -al;echo\\\""
  ]
}

The above JSON file results in the output:
Includes are: ""
, followed by a directory listing (an actual attack would probably be something far more malicious).

What I'd like instead is something like the following to be outputted:
Includes are: "\\\";ls -al;echo\\\""

Edit 1

I used echo as an example command in the script, which probably wasn’t the best example, as then the solution is simply not using eval.

However the actual command that will be needed is dotnet test, and each array item from Includes needs to be passed as an option using /p:<Includes item>. What I was hoping for was a way to globally neutralise injection regardless of the command, but perhaps that’s not possible, ie, the technique you go for relies heavily on the actual command.

8
  • 3
    If all you want is to print the contents of the Include array, why are you evaling (twice)? Apr 8, 2019 at 14:49
  • 2
    don't use eval. problem solved Apr 8, 2019 at 15:11
  • INCLUDES=( $(jq '.Include[]' user-supplied.json) ) does what you want, without executing any user-supplied input. You may need to adjust some of jq's options to get the shell array that you want, but nothing requiring eval.
    – chepner
    Apr 8, 2019 at 15:18
  • @BenjaminW Please see Edit 1. Is it still possible to not use eval?
    – AjLearning
    Apr 8, 2019 at 22:27
  • @oguzismail As above ^.
    – AjLearning
    Apr 8, 2019 at 22:27

2 Answers 2

2

You don't need to use eval for dotnet test either. Many bash extensions not present in POSIX sh exist specifically to make eval usage unnecessary; if you think you need eval for something, you should provide enough details to let us explain why it isn't actually required. :)

#!/usr/bin/env bash
#              ^^^^- Syntax below is bash-only; the shell *must* be bash, not /bin/sh

include_args=( )
IFS=$'\n' read -r -d '' -a includes < <(jq -r '.Include[]' user-supplied.json && printf '\0')
for include in "${includes[@]}"; do
  include_args+=( "/p:$include" )
done

dotnet test "${include_args[@]}"

To speak a bit to what's going on:

  • IFS=$'\n' read -r -d '' -a arrayname reads up to the next NUL character in stdin (-d specifies a single character to stop at; since C strings are NUL-terminated, the first character in an empty string is a NUL byte), splits on newlines, and puts the result into arrayname.

    The shorter way to write this in bash 4.0 or later is readarray -t arrayname, but that doesn't have the advantage of letting you detect whether the program generating the input failed: Because we have the && printf '\0' attached to the jq code, the NUL terminator this read expects is only present if jq succeeds, thus causing the read's exit status to reflect success only if jq reported success as well.

  • < <(...) is redirecting stdin from a process substitution, which is replaced with a filename which, when read from, returns the output of running the code ....
  • The reason we can set include_args+=( "/p:$include" ) and have it be exactly the same as include_args+=( /p:"$include" ) is that the quotes are read by the shell itself and used to determine where to perform string-splitting and globbing; they're not persisted in the generated content (and thus later passed to dotnet test).

Some other useful references:

  • BashFAQ #50: I'm trying to put a command in a variable, but the complex cases always fail! -- explains in depth why you can't store commands in strings without using eval, and describes better practices to use instead (storing commands in functions; storing commands in arrays; etc).
  • BashFAQ #48: Eval command and security issues -- Goes into more detail on why eval is widely frowned on.
12
  • Thanks Charles, I’ll try this and accept if it works.
    – AjLearning
    Apr 8, 2019 at 22:54
  • I’m getting a “syntax error near unexpected token ‘<‘ “ ?
    – AjLearning
    Apr 8, 2019 at 23:59
  • I've tried to expand with explanation and references. Let me know if there's anything more that would be needed to constitute a complete and full answer. Apr 9, 2019 at 0:35
  • Thanks for the additional info. It works. Just had to make one change which is remove the -r option from the call to jq, as the quotes are needed for the argument to /p:, which has the form /p:<key>="<include>".
    – AjLearning
    Apr 9, 2019 at 0:38
  • @AjLearning, when you're entering it at bash, do you do /pkey="include", or '/pkey="include"'? When content is evaluated as code (as when it's manually typed, but not when it comes from a parameter expansion), unless you escape your double quotes (as by putting single-quotes around them or backslashes just before them), they just get removed by bash and not passed to dotnet test at all. Apr 9, 2019 at 14:55
0

You don't need eval at all.

INCLUDES=( $(jq '.Include[]' user-supplied.json) )
echo "Includes are: "
for e in "${INCLUDES[@]}"; do
  echo "$e"
done

The worst that can happen is that the unquoted command substitution may perform word-splitting or pathname expansion where you don't want it (which is a problem in your original as well), but there's no possibility for arbitrary command execution.

5
  • Is there a particular reason to use string-splitting here, vs readarray -t includes < <(jq -r ...)? (For other readers of this comment, see BashPitfalls #50 for context). Apr 8, 2019 at 15:45
  • I'm punting on the question of what INCLUDES should actually contain. It's quite possible that no array is even needed for whatever the OP ultimately wants.
    – chepner
    Apr 8, 2019 at 15:53
  • @chepner Please see Edit 1.
    – AjLearning
    Apr 8, 2019 at 22:29
  • @CharlesDuffy Perhaps I’m missing something since my Bash skills aren’t that great, but I thought eval is needed to execute a built up string? The full command string that’s built might end up looking like: dotnet test /p:”item1” /p:”item2”, where each of the items comes from Includes.
    – AjLearning
    Apr 8, 2019 at 22:38
  • 1
    @AjLearning, you shouldn't build commands as strings at all, you should build them as arrays. See BashFAQ #50 and BashFAQ #48. Apr 8, 2019 at 22:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.