specification of strstr in dracut-lib

[jcflack]

In C, the strstr() function is a literal substring match, not a pattern match; in dracut-lib, there is a strstr() function, but it will perform a pattern match if the search argument contains shell glob characters *?[...]. The comments in dracut-lib do not say it is a pattern match, and perhaps it was not intended to be. My first instinct would be to call it a bug, and fix it to be a literal substring match. As it is, modules may be prone to unwanted behavior if certain characters are present in input, if the module authors assumed it was a safe substring match.

However, there are about 100 uses of strstr throughout the dracut source tree. Most of them use it as a literal string match (as far as I can see from a lazy grep). But there are about eleven places where code actually depends on strstr being a pattern match:

./modules.d/40network/net-lib.sh:    if strstr "$autoconf" "*.*.*.*"; then
./modules.d/40network/ifup.sh:    strstr $ip '*:*:*' && load_ipv6
./modules.d/40network/ifup.sh:    if strstr $ip '*:*:*'; then
./modules.d/95nfs/nfs-lib.sh:    if strstr "$arg" ':/*' ; then
./modules.d/99base/dracut-lib.sh:    if strstr "$arg" ':/*' ; then
./modules.d/80cms/cms-write-ifcfg.sh:    strstr "$IPADDR" '*:*:*' && ipv6=1
./modules.d/80cms/cmsifup.sh:strstr "$IPADDR" '*:*:*' && ipv6=1
./modules.d/45ifcfg/write-ifcfg.sh:    strstr "$netif" ":*:*:*:*:" && continue
./modules.d/45ifcfg/write-ifcfg.sh:            if strstr "$ip" '*:*:*'; then
./modules.d/45ifcfg/write-ifcfg.sh:            if strstr "$gw" '*:*:*'; then
./test/TEST-50-MULTINIC/client-init.sh: strstr "$i" ":*:*:*:*:" && continue

So there's a dilemma. Fixing strstr to be a literal string match would definitely break about 11% of uses in the dracut source tree (and some unknown number of uses in modules that might be maintained elsewhere) - but it would protect the other 89% from possible breakage by unexpected input (and some unknown number maintained out-of-tree).

Leaving it as is, and documenting that it is a glob match, would avoid breaking the 12% that use it that way, and eventually people could inspect the other 88% for the risk of misbehavior with strange input.

Either way, people who maintain modules (especially out of the dracut source tree) would need to know the decision, so they could inspect or update their code.

My proposal would probably be to rename the existing strstr to something like strglob, and change the known existing sites that use it that way to use strglob, and define a new strstr that is a literal match.

That's probably what I would do, but I won't presume to make the call.

There are also str_starts and str_ends that are glob matches but do not say so, but I did not find any instances of them being used that way, so it is probably an easier call to simply fix them to do literal matching.

I have a start on patches in a topic branch, but before making a pull request I wanted to check what the preferred approach would be.

[haraldh]

Rename to strglob() and introduce a new strstr() is fine. I will then announce this change on the mailing list, so that module writers, which rely on the glob could change their code.