QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts - Debian

This is a discussion on QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts - Debian ; Hi, many wrapper scripts contain things like export LD_LIBRARY_PATH=foo:$LD_LIBRARY_PATH This is bad because if LD_LIBRARY_PATH is unset, it will expand to LD_LIBRARY_PATH=foo: which is interpreted as LD_LIBRARY_PATH=foo:. This means that the current directory is searched for libraries before /lib and ...

+ Reply to Thread
Results 1 to 5 of 5

Thread: QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

  1. QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

    Hi,

    many wrapper scripts contain things like

    export LD_LIBRARY_PATH=foo:$LD_LIBRARY_PATH

    This is bad because if LD_LIBRARY_PATH is unset, it will expand to

    LD_LIBRARY_PATH=foo:

    which is interpreted as

    LD_LIBRARY_PATH=foo:.

    This means that the current directory is searched for libraries before
    /lib and /usr/lib, which can have security implications.

    The fix is to use "${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" instead of
    ":$LD_LIBRARY_PATH". This will get rid of the colon if LD_LIBRARY_PATH
    is unset. (Actually, some scripts use "${LD_LIBRARY_PATH+:
    $LD_LIBRARY_PATH}", which seems to work, too. But this is not
    documented in the bash man page, at least I can't find it.)

    This is not a new issue: CVE-2005-4790 and CVE-2005-4791 have been
    found two years ago. Unfortunately, they were first announced as SuSE
    specific packaging errors and were missed by the security teams.

    I filed #451548 for liferea, but many more packages are affected. I
    intend to file a wishlist bug for lintian to check for this. But
    since this will take some time to get implemented, if someone has a
    local mirror and wants to do some QA work, a complete check of the
    archive would be good.

    Of course "$LD_LIBRARY_PATH:" is just as bad as ":$LD_LIBRARY_PATH".
    Maybe there are other environment variables that could be affected by
    the same problem. For $PATH it is not a problem, because it should
    always be set. More ideas?


    Cheers,
    Stefan


    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.6 (GNU/Linux)

    iD8DBQBHPgU0bxelr8HyTqQRArd3AJ4/MKnN1Eh9B93509WvNOAg/pkrRgCePior
    ZSNoggoEsBG3agdyBFxfn8c=
    =+H2e
    -----END PGP SIGNATURE-----


  2. Re: QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

    On Fri, 16 Nov 2007 22:01:34 +0100, Stefan Fritsch wrote:
    [...]
    > The fix is to use "${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" instead of
    > ":$LD_LIBRARY_PATH". This will get rid of the colon if LD_LIBRARY_PATH
    > is unset. (Actually, some scripts use "${LD_LIBRARY_PATH+:
    > $LD_LIBRARY_PATH}", which seems to work, too. But this is not
    > documented in the bash man page, at least I can't find it.)


    Actually it is documented, just above the description of the various
    expansions:
    bash tests for a parameter that is unset or null; omitting the colon
    results in a test only for a parameter that is unset.

    Given that a null LD_LIBRARY_PATH seems to have no effect, just as if
    it were unset, the :+ form appears to be more appropriate.

    --
    Micha³ Politowski
    Talking has been known to lead to communication if practiced carelessly.

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.6 (GNU/Linux)

    iD8DBQFHPgoUfe5k6+27tW8RAgXoAJwLltqYAj7zYhkssrBI/86sHF88hACfaUzR
    rFZYUCob/6tspvIXUH/dROU=
    =rPjd
    -----END PGP SIGNATURE-----


  3. Re: QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

    On Fri, Nov 16, 2007 at 10:01:34PM +0100, Stefan Fritsch wrote:
    > Hi,
    >
    > many wrapper scripts contain things like
    >
    > export LD_LIBRARY_PATH=foo:$LD_LIBRARY_PATH
    >
    > This is bad because if LD_LIBRARY_PATH is unset, it will expand to
    >
    > LD_LIBRARY_PATH=foo:
    >
    > which is interpreted as
    >
    > LD_LIBRARY_PATH=foo:.
    >
    > This means that the current directory is searched for libraries before
    > /lib and /usr/lib, which can have security implications.
    >
    > The fix is to use "${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" instead of
    > ":$LD_LIBRARY_PATH". This will get rid of the colon if LD_LIBRARY_PATH
    > is unset. (Actually, some scripts use "${LD_LIBRARY_PATH+:
    > $LD_LIBRARY_PATH}", which seems to work, too. But this is not
    > documented in the bash man page, at least I can't find it.)
    >
    > This is not a new issue: CVE-2005-4790 and CVE-2005-4791 have been
    > found two years ago. Unfortunately, they were first announced as SuSE
    > specific packaging errors and were missed by the security teams.
    >
    > I filed #451548 for liferea, but many more packages are affected. I
    > intend to file a wishlist bug for lintian to check for this. But
    > since this will take some time to get implemented, if someone has a
    > local mirror and wants to do some QA work, a complete check of the
    > archive would be good.
    >
    > Of course "$LD_LIBRARY_PATH:" is just as bad as ":$LD_LIBRARY_PATH".
    > Maybe there are other environment variables that could be affected by
    > the same problem. For $PATH it is not a problem, because it should
    > always be set. More ideas?


    Are there real use cases for having ":something" or "something:" as
    $LD_LIBRARY_PATH ? Are there applications relying on LD_LIBRARY_PATH
    taking empty parts and acting as if they were '.' ?
    Wouldn't it be just better to change the dynamic loader so that it
    ignores empty parts of the LD_LIBRARY_PATH ? That would solve the
    problem once and for all, and avoid people to shoot themselves in the foot
    by writing "$LD_LIBRARY_PATH:foo" (which they shouldn't, but reality is
    not everyone is aware of the problem).

    Mike


    --
    To UNSUBSCRIBE, email to debian-security-REQUEST@lists.debian.org
    with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

  4. Re: QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

    * Stefan Fritsch (sf@debian.org) [071116 13:03]:
    > is unset. (Actually, some scripts use "${LD_LIBRARY_PATH+:
    > $LD_LIBRARY_PATH}", which seems to work, too. But this is not
    > documented in the bash man page, at least I can't find it.)


    The difference between ${PARAMETER:+WORD} and ${PARAMETER+WORD} is
    subtle, and you're right, it's not documented in the bash man page.
    It is part of the POSIX shell standard, though. ${PARAMETER:+WORD}
    substitutes WORD if PARAMETER is set and non-empty. ${PARAMETER+WORD}
    substitutes WORD if PARAMETER is set, empty or not. For example:

    vineet@sprocket:~$ FOO=
    vineet@sprocket:~$ echo ${FOO+BAR}
    BAR
    vineet@sprocket:~$ echo ${FOO:+BAR}

    vineet@sprocket:~$ unset FOO
    vineet@sprocket:~$ echo ${FOO+BAR}

    vineet@sprocket:~$ echo ${FOO:+BAR}

    vineet@sprocket:~$

    In many cases they'll be equivalent, but in the LD_LIBRARY_PATH case,
    I'd recommend using the colon-form. If someone has set an empty
    LD_LIBRARY_PATH, the correct behavior is just to add the directory you
    want; you don't want to stick an extra empty pathname component in
    there.

    good times,
    Vineet

    --
    http://www.doorstop.net/
    --
    "As we enjoy great advantages from inventions of others, we should be glad
    of an opportunity to serve others by any invention of ours; and this we
    should do freely and generously." --Benjamin Franklin

    -----BEGIN PGP SIGNATURE-----
    Version: GnuPG v1.4.6 (GNU/Linux)

    iD8DBQFHQoua7z3S33fUb9ERAj8hAKDJ/m0O/oKdcz9Lm/lhUeoKTZ0SZwCeOqS0
    N0ZZq6FfQWyeGgrMwZ0ZikM=
    =BAq8
    -----END PGP SIGNATURE-----


  5. Re: QA needed for insecure LD_LIBRARY_PATH in many wrapper scripts

    Vineet Kumar wrote:
    > * Stefan Fritsch (sf@debian.org) [071116 13:03]:
    >> is unset. (Actually, some scripts use "${LD_LIBRARY_PATH+:
    >> $LD_LIBRARY_PATH}", which seems to work, too. But this is not
    >> documented in the bash man page, at least I can't find it.)

    >
    > The difference between ${PARAMETER:+WORD} and ${PARAMETER+WORD} is
    > subtle, and you're right, it's not documented in the bash man page.


    >From the bash manpage:

    In each of the cases below, word is subject to tilde expansion, parameā€
    ter expansion, command substitution, and arithmetic expansion. When
    not performing substring expansion, bash tests for a parameter that is
    unset or null; omitting the colon results in a test only for a parameā€
    ter that is unset.

    But I agree that I would not have found it if I did not know what to
    search.

    > It is part of the POSIX shell standard, though.


    Yes, and posh has it. I prefer the formulation from the posh manpage:
    In the above modifiers, the : can be omitted, in which case the
    conditions only depend on name being set (as opposed to set and not
    null). [...]

    Vincent

    --
    Vincent Danjean GPG key ID 0x9D025E87 vdanjean@debian.org
    GPG key fingerprint: FC95 08A6 854D DB48 4B9A 8A94 0BF7 7867 9D02 5E87
    Unofficial pacakges: http://www-id.imag.fr/~danjean/deb.html#package
    APT repo: deb http://perso.debian.org/~vdanjean/debian unstable main


    --
    To UNSUBSCRIBE, email to debian-security-REQUEST@lists.debian.org
    with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org

+ Reply to Thread