bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Incorrect / Inconsistent behavior with nameref assignments in functi


From: Koichi Murase
Subject: Re: Incorrect / Inconsistent behavior with nameref assignments in functions
Date: Mon, 31 Aug 2020 03:01:55 +0900

2020-08-30 19:24 Binarus <lists@binarus.de>:
> Actually, this is what first happened to me and what led me to the
> problem described in my original post.
>
> [...]

Thank you for your explanation! Now I see your situation.

>> * Another way is to copy to the local array only when the name is
>>   different from `myArray':
>>
>>   [...]
>
> However, eval is evil. If I ever had to provide that function to
> other users (which currently is not the case), then I would have a
> problem if another user would call it like that:

Yes, I recognize the problem when the function isn't properly used.
But, the use of eval itself is not fatal.  When another user can call
the function as in your example,

  Dummy 'myArray1[@]}"); echo Gotcha!; #'

that means that the user can already run arbitrary commands.  The user
could have directly written

  echo 'Gotcha!'

The real problems occur when the user write like

  Dummy "$input_to_program"

with `input_to_program' provided by the third user who should not be
able to run arbitrary commands, and the input is not checked nor
sanitized.  In this case, the problem should be evaded by checking or
sanitizing the input.  This check can be made inside the function
Dummy, but it is also possible to make it at the time when the shell
receives the input.

> declare -a -i myArray1=('1' '2' '3')
> Dummy 'myArray1[@]}"); echo Gotcha!; #'
>
> Output:
>
> root@cerberus:~/scripts# ./test6
> Gotcha!
> declare -a myArray=([0]="1" [1]="2" [2]="3")

Unfortunately, your original function `Dummy' also has the same
vulnerability.  As Greg has written, there are many other places that
cause the command execution in the shell because that is the purpose
of the shell.  With your original method,

  $ cat testR2d.sh
  function Dummy {
    local -n namerefArray="$1"
    local -a -i myArray=("${namerefArray[@]}")
  }
  myArray1=("$@")
  Dummy 'myArray1'
  $ bash testR2d.sh 'a[$(echo Gotcha1 >/dev/tty)]'
  Gotcha1

This is caused by the arithmetic evaluation caused by `-i' flag.  In
arithmetic evaluations, the array subscripts are subject to the extra
expansions so that the string $(echo Gotcha1 >/dev/tty) is expanded as
a command substitution.

Actually, the nameref also has the same behavior, so the use of
`nameref' is not much safer than the use of `eval'.

  $ cat testR2e.sh
  function Dummy2 {
    local -n namerefScalar=$1
    local var=$namerefScalar
  }
  Dummy2 "$1"
  $ bash testR2e.sh 'a[$(echo Gotcha2 >/dev/tty)]'
  Gotcha2

Yes, I guess it would be a valid strategy to disallow any use of
`eval' because humans will make mistakes no matter how careful we are.
But, there are still different traps, so anyway we need to carefully
check or sanitize inputs even when we don't use `eval'.

> I guess it would be very complicated, if possible at all, to protect
> the code inside eval against every sort of such attacks.

I think the standard way is to check the input before passing it to
`eval' and is not complicated.  You can just check if the array name
has an expected form:

  function is-valid-array-name {
    local reg='^[_[:alpha:]][_[:alnum:]]*$'
    [[ $1 =~ $reg ]]
  }

  # Check it inside Dummy
  function Dummy {
    is-valid-array-name "$1" || return 1
    [[ $1 == myArray ]] || eval "local -a myArray=(\"\${$1[@]}\")"
    declare -p myArray
  }

  # Or check it when it receives the array name (I prefer this)
  is-valid-array-name "$1" || exit 1
  input_data=$1
  Dummy "$input_data"

>> * If you want to use namerefs to eliminate the use of `eval', maybe
>>   you could do like the following [...]
>
> However (hoping that I don't get flamed for a dumb question), I
> don't understand why we need inputArray at all in that code.

Sorry, I should have explained it in detail.  The step of `inputArray'
is only needed when you want to modify `myArray' locally in the
function `Dummy' keeping the original array unmodified.  Without
`inputArray',

  $ cat testR2f.sh
  function Dummy {
    [[ $1 == refArray ]] || local -n refArray=$1
    [[ $1 == myArray ]] || local -ia myArray=("${refArray[@]}")
    myArray[0]=${myArray[0]%/}
  }
  myArray=(my/dir/)
  declare -p Dummy
  Dummy myArray
  declare -p Dummy
  $ bash testR2f.sh
  declare -a myArray=([0]="my/dir/")
  declare -a myArray=([0]="my/dir")

This is because, when the outer array has the same name `myArray', the
function Dummy sees the outer array directly instead of the local
copy.

> Wouldn't the following function be sufficient?
>
> function Dummy {
>   [[ $1 == refArray ]] || local -n refArray=$1
>   local -ia myArray=("${refArray[@]}")
>   declare -p myArray
> }

No, that function solves the problem of the collision with `refArray'
(the circular reference) but not the problem of the collision with
`myArray' (the problem in your original post).

> Unfortunately, these solutions (while solving the circular reference
> problem) don't solve my original problem.

Have you tried?  In the examples in my previous reply, I intended to
solve both problems with older versions of Bash.

> I think I'll stick with my current (extremely ugly, but reliable)
> solution

Yes, I think that is the simplest solution in your case.


Best regards,

Koichi



reply via email to

[Prev in Thread] Current Thread [Next in Thread]