fedora-review: 'Illegal return' warnings

[Florian Weimer]

On 10/04/2014 10:18 PM, Alec Leamas wrote:
> Hm.... seems that recent bash patch to fix the shellshock problem
> introduces this. Fedora-review relies on exported shell functions
> (export -f) and the bash fix changes the syntax for exported functions
> in an incompatible way.

It's the attempt at cleaning up the environment, see 
/usr/share/fedora-review/plugins/shell_api.py:

unset $(env | sed -n 's/=.*//p')

With exported functions, that was fairly broken before (with multi-line 
function definitions and “=” somewhere in the body), but after the bash 
change, this is much more obvious and is even triggered by the exported 
function in the environment-modules package.  It would have been 
preferable to clean the environment either in the Python code, or wrap 
the shell invocation with “env -i”.

I still hope we can agree with upstream on another bash change which 
hides these bugs again, but it's difficult to separate this aspect from 
the security/hardening discussions which generate much more interest, 
overshadowing anything else.  (Upstream's “%%” would have generated 
errors here as well.)

By the way, it doesn't seem to me fedora-review needs exported 
functions, the function definitions are sourced as needed, so they don't 
have to be in the environment.

-- 
Florian Weimer / Red Hat Product Security