Re: fedora-review: 'Illegal return' warnings

[Alec Leamas]

On 2014-10-06 15:16, Florian Weimer wrote:

On 10/05/2014 05:15 PM, Florian Weimer wrote:

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”.


And indeed this had already been reported as a bug, completely 
unrelated to exported functions:


  



"blushes"

I shall try to have a  look at this. MIght take some time, though.

--alec
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct

Re: fedora-review: 'Illegal return' warnings

[Florian Weimer]

On 10/05/2014 05:15 PM, Florian Weimer wrote:

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”.


And indeed this had already been reported as a bug, completely unrelated 
to exported functions:


  

--
Florian Weimer / Red Hat Product Security
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct

Re: 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
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct

Re: fedora-review: 'Illegal return' warnings

[Alec Leamas]

On 2014-10-04 20:12, Antonio Trande wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi all.

These warnings appear during a package review; apparently,
fedora-review command completes all its tasks.

WARNING: Illegal return from
/usr/share/fedora-review/scripts/generic-excludearch.sh, code 82,
output: stdout:None stderr:./review-env.sh: line 7: unset:
`BASH_FUNC_module()': not a valid identifier


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.


For now, this means that you should check these things manually:
- Nothing under /srv, /opt or /usr/local
- Package (or it's dependencies) is not known to require an ExcludeArch tag.
- Large data in /usr/share should live in a noarch subpackage.
- Large (or many) docs must go in a -doc subpackage.

Besides this, you can safely ignore these errors.

For fedora-review we need to evaluate what is reqiured to stay 
compatible with bash after this Shellshock issue.


Cheers!

--alec
--
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct

fedora-review: 'Illegal return' warnings

[Antonio Trande]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Hi all.

These warnings appear during a package review; apparently,
fedora-review command completes all its tasks.

WARNING: Illegal return from
/usr/share/fedora-review/scripts/generic-excludearch.sh, code 82,
output: stdout:None stderr:./review-env.sh: line 7: unset:
`BASH_FUNC_module()': not a valid identifier

WARNING: Illegal return from
/usr/share/fedora-review/scripts/generic-large-docs.sh, code 82,
output: stdout:Documentation size is 30720 bytes in 3 files.
stderr:./review-env.sh: line 7: unset: `BASH_FUNC_module()': not a
valid identifier

WARNING: Illegal return from
/usr/share/fedora-review/scripts/generic-srv-opt.sh, code 80, output:
stdout:None stderr:./review-env.sh: line 7: unset:
`BASH_FUNC_module()': not a valid identifier

WARNING: Illegal return from
/usr/share/fedora-review/scripts/generic-large-data.sh, code 80,
output: stdout:None stderr:./review-env.sh: line 7: unset:
`BASH_FUNC_module()': not a valid identifier

What they are?

- -- 
Antonio Trande

mailto: sagitterATfedoraproject.org
http://fedoraos.wordpress.com/
https://fedoraproject.org/wiki/User:Sagitter
GPG Key: 0x66E15D00
Check on https://keys.fedoraproject.org/
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIcBAEBAgAGBQJUMDh/AAoJEFyovWBm4V0ATB4P/ituuJrGN4qRIZsrrRU3y/w/
51hiYUB+68XmfDfdun3PO3EEJMZkm1KfZ7cSqOhAfziUFhviYlkS4T43pSE+2fRT
WjC4vRsfXSlsRNnTSBikV7iUZjCPqSHsINNucHiMkUJtumfDoN48RqhwgCqqes2V
PaHiV0JTsC30hicle8xrMdAQeVchvTODz4wpiUNUJAKfQY0Ha5VisymRB1At1G+5
13xSVvbsUCqUW389jg2NAXzoHKcONfQIO0Do+ud6KSfKpP1yNWsiKZlb/ML166iw
aVDnA1BRR5e5EPPYKJ19z4G+GJ+DxBMMvhVAFFw2qOXheJa+XMrFASBWGBN0cCKA
0ApgSo6QAMDDnmvTCLGbEZSLv/ETaAvBpKOchAJQIeMtY8agxr0D5v6GOyh/IgC4
QSZNfVmnn7A1m2iNZLX+fDXraf5LX88qODdDcTcy2u6ZcZDvO/I+l5QXuQKvNPeg
6dtaylrQ/BmQHJrpJfZXeefUjBFZQCr+A8wxsddEIV8DVBb2oTI4B9qtO95xbAO7
uzSM2tpwsH2g9iRElmNzbFXMtOquVojAfiFt5JqTieyo68NmM2psf65k5ZdICVIC
huY3FsbCyv+HqJWekOp7TipiRcl8+dh8SlTntdsxW16PhpwNMidA3a01TvhYE6g2
ATA7fKMvYrmmeJUzNCdT
=fR6X
-END PGP SIGNATURE-
-- 
devel mailing list
devel@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/devel
Fedora Code of Conduct: http://fedoraproject.org/code-of-conduct