Bug 241608

Summary: typo/bug in /usr/sbin/bind-chroot-admin
Product: [Fedora] Fedora Reporter: D. Hugh Redelmeier <hugh>
Component: bindAssignee: Adam Tkac <atkac>
Status: CLOSED DUPLICATE QA Contact: Ben Levenson <benl>
Severity: medium Docs Contact:
Priority: medium    
Version: 6CC: ovasik
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-05-29 11:33:22 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description D. Hugh Redelmeier 2007-05-28 21:09:06 UTC
Description of problem:
/usr/sbin/bind-chroot-admin has an obvious typo.  Others have observed errors
from running the script:

/usr/sbin/bind-chroot-admin: line 224: [: : unary operator expected

https://www.redhat.com/archives/fedora-list/2007-May/msg02528.html

Version-Release number of selected component (if applicable):
bind-9.3.4-5.fc6

How reproducible:
Always

Steps to Reproduce:
1. read the code
  
Additional info:
Line 224 reads:
    if [ "$ENABLE_ZONE_WRITE" =  [yY1]* ]; then
It should probably read:
    if [ "$ENABLE_ZONE_WRITE" : '[yY1]*' ]; then

The current test is bogus in several ways.  Unless the man page expr(1) is
bogus (never trust GNU-generated man pages).

The shell command "[" is supposed to be another name for "expr".
In expr, the binary operator "=" does not take regular expressions.
So the "[yY1]*" makes no sense.  The test does not do what the author
appears to think it will do.

But it is worse than that.  The shell does globbing on this operand
since it isn't quoted.  So [yY1]* would get expanded to the list of
filenames in the current directory whose names start with y, Y, or 1.
If none match it would remain unchanged.  The result would be passed
to the expr.  If it were a list, that would be a mess.  If it were a
single item, it would just be wrong.

Clearly the then path through this if has never been tested properly.

Comment 1 D. Hugh Redelmeier 2007-05-29 04:33:09 UTC
Oops -- [ is a version of test(1) not expr(1).  So that the fixed line should be
    if expr "$ENABLE_ZONE_WRITE" : '[yY1]*' ; then

A more traditional replacement would be a case statement replacing the whole if:

        case "$ENABLE_ZONE_WRITE" in
        [yY1]*) return 0 ;;
        esac


Comment 2 D. Hugh Redelmeier 2007-05-29 05:07:57 UTC
The test(1) operator = is correct and == is a BASHism.  For maximum portability,
line 215 should use = instead of ==.

At some point the script might be run by ash/dash/busybox (perhaps for
performance reasons).  The dash manual that I have does not document ==.

(The expr command isn't listed as a dash builtin so that is one reason to use
the case formulation I mentioned above.)


Comment 3 Adam Tkac 2007-05-29 11:33:22 UTC
Sorry for problems, already fixed in 9.3.4-6.fc6
(http://people.redhat.com/atkac/bind/). I could change == to = in rawhide. But I
don't think that this is absolutely necessary because chroot-admin is bash script.

Regards, Adam


*** This bug has been marked as a duplicate of 241103 ***