bugfindutils - Bugs: bug #19374, Insufficient quoting of PRUNEPATHS...

 
 

bug #19374: Insufficient quoting of PRUNEPATHS in updatedb

Submitter:  Andreas Metzler <ametzler>
Submitted:  Wed 21 Mar 2007 06:21:47 PM UTC
   
 
Category:  updatedb Severity:  3 - Normal
Item Group:  None Status:  Postponed
Privacy:  Public Assigned to:  None
Originator Name:  István Váradi Open/Closed:  Open
Release:  4.2.28 Fixed Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 01 Aug 2023 06:25:30 PM UTC, comment #6: 


> -val=`echo $arg|sed 's/^[^=]*=\(.*\)/\1/'` || exit 71
> +val=`echo "$arg"|sed 's/^[^=]*=\(.*\)/\1/'` || exit 71


This seems correct, but should also be applied to the line before for consistency:

-  opt=`echo $arg|sed 's/^\([^=]*\).*/\1/'`  || exit 71
+  opt=`echo "$arg"|sed 's/^\([^=]*\).*/\1/'`  || exit 71


> - for p in $PRUNEPATHS; do
> + for p in "$PRUNEPATHS"; do


This defeats the purpose of the for-loop, as this tries to check if any of
the paths in PRUNEPATHS ends on '/', while the patch would reduce that check
only to the last element (due to quoting into a single string).

I'd suggest to change this to:

-for p in $PRUNEPATHS; do
-    case "$p" in
-       /*/)   echo "$0: $p: pruned paths should not contain trailing slashes" >&2
-              exit 1
-    esac
-done
+nl='
+'
+if echo "$PRUNEPATHS" | tr ' ' "$nl" | grep '[^/]/$' >/dev/null; then
+  echo "$0: $p: pruned paths should not contain trailing slashes" >&2
+  exit 1
+fi


> test -z "$PRUNEREGEX" &&
> -  PRUNEREGEX=`echo $PRUNEPATHS|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`
> +  PRUNEREGEX=`echo "$PRUNEPATHS"|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`


I see several problems in this area.

updatedb allows to define PRUNEREGEX as environment variable from outside,
and only uses the value of --prunepath if the former is unset.
Usually, options should override environment variables, not the other way round.

Furthermore, PRUNEPATHS seems to be defined to allow already-expanded items only.
If one wants to use regular expressions, then why not directly define PRUNEREGEX from outside?

In that regard, it would maybe be better to introduce a --pruneregex option
which takes the final pruning expression for find(1).  It could still override
any of PRUNEPATHS and --prunepaths value, but shouldn't do that silently.

Bernhard Voelker <berny>
Group administrator
Wed 26 Jul 2023 12:01:38 PM UTC, comment #5: 

Indeed, the patches don't work because they miss a critical unquoted variable, "arg".

The following patch should work:

-val=`echo $arg|sed 's/^[^=]*=\(.*\)/\1/'` || exit 71
+val=`echo "$arg"|sed 's/^[^=]*=\(.*\)/\1/'` || exit 71
[...]
- for p in $PRUNEPATHS; do
+ for p in "$PRUNEPATHS"; do
[...]
-  PRUNEREGEX=`echo $PRUNEPATHS|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`
+   PRUNEREGEX=`echo "$PRUNEPATHS"|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`

Sebastian Carlos <sebastiancarlos>
Wed 14 Jul 2010 08:11:50 PM UTC, comment #4: 

Patches don't work for me when using the command-line option --prunepath as the asterisks are expanded in the "for arg" access of the implicit $@.  As a workaround, wrapping my pattern with \( and \) works, like updatedb --prunepaths='\(.*/.svn\)'

Patches do work when setting the PRUNEPATH environment variable.

Anonymous
Sun 11 Apr 2010 11:56:38 AM UTC, comment #3: 

updatedb would probably be best rewritten.   I'm going to mark the updatedb-related bus as "Postponed" in order to consider them all in a batch, when I get around to the rewrite.

(Leslie P. Polzer did start on a rewrite some time back as part of the Summer of Code but there are some updatedb-related issues his changes don't resolve yet, I think)

James Youngman <jay>
Group administrator
Mon 09 Feb 2009 09:22:17 AM UTC, comment #2: 

Here is a patch:

--- updatedb.org        2009-01-27 13:29:28.575086300 +0100
+++ updatedb    2009-02-09 10:19:41.132505500 +0100
@@ -163,7 +163,7 @@
 # Trailing slashes result in regex items that are never matched, which
 # is not what the user will expect.   Therefore we now reject such
 # constructs.
-for p in $PRUNEPATHS; do
+for p in "$PRUNEPATHS"; do
     case "$p" in
        /*/)   echo "$0: $p: pruned paths should not contain trailing slashes" >&2
               exit 1
@@ -172,7 +172,7 @@
 
 # The same, in the form of a regex that find can use.
 test -z "$PRUNEREGEX" &&
-  PRUNEREGEX=`echo $PRUNEPATHS|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`
+  PRUNEREGEX=`echo "$PRUNEPATHS"|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`
 
 # The database file to build.
 : ${LOCATE_DB=/var/locatedb}

Anonymous
Tue 27 Jan 2009 12:21:56 PM UTC, comment #1: 

I second this ticket. Trying to prune directories based on regular expressions doesn't work. For example:

updatedb --findoptions='-mount' --localpaths='/cygdrive/c' --prunepaths='.*/.svn'

The asterisk in '.*/.svn' is shell-expanded by the line where pruned paths are converted to regular expressions. If $PRUNEPATH in this line is placed in double quotes works.

Anonymous
Wed 21 Mar 2007 06:21:47 PM UTC, original submission:  

This is http://bugs.debian.org/415567

István Váradi writes: -----------------------
The updatedb script uses the PRUNEPATHS environment variable without quoting it. This causes some problems when putting regular expressions into the value of this variable. For example, if I use an asterisk () in a path, it will be expanded by the shell before using it as a regular expression. Thus, ./lost+found becomes ../lost+found (and possibly others, depending on how many files or directories, whose names start with a dot I have in the working directory).
---------------------------------------------

Afaict the fix should be trivial:

- for p in $PRUNEPATHS; do
+ for p in "$PRUNEPATHS"; do
[...]
-  PRUNEREGEX=`echo $PRUNEPATHS|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`
+   PRUNEREGEX=`echo "$PRUNEPATHS"|sed -e 's,^,\\\(^,' -e 's, ,$\\\)\\\|\\\(^,g' -e 's,$,$\\\),'`

thanks, cu andreas

Andreas Metzler <ametzler>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by berny (Posted a comment)
  • -email is unavailable- added by sebastiancarlos (Posted a comment)
  • -email is unavailable- added by jay (Posted a comment)
  • -email is unavailable- added by ametzler
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-04-11 jay StatusNone Postponed
    2007-03-21 ametzler Carbon-CopyRemoved 20807 -
    2007-03-21 ametzler Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code