Bug 18443 - Fehlendes Escaping für eval "$(ucr shell)"
Fehlendes Escaping für eval "$(ucr shell)"
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UCR
UCS 2.3
All Linux
: P4 normal (vote)
: UCS 3.1
Assigned To: Lukas Walter
Sönke Schwardt-Krummrich
: interim-1
: 23985 (view as bug list)
Depends on: 8417
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-20 10:20 CEST by Philipp Hahn
Modified: 2012-12-12 21:09 CET (History)
4 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Hahn univentionstaff 2010-05-20 10:20:30 CEST
In vielen Skripten wird nur
 eval $(ucr shell)
statt
 eval "$(ucr shell)"
verwendet. Dies führt dazu, daß nach der Expansion von "ucr shell" die Variablenzuweisungen nochmals durch die Shell expandiert werden. Das folgende Beispiel zeigt das Problem:
# echo *
sources.list
# ucr set test="* * *"
Create test
# ucr shell test
test="* * *"
# eval $(ucr shell test)
# echo "$test"
* sources.list *
# eval "$(ucr shell test)"
# echo "$test"
* * *

Das ist insbesondere UCR-Variablen mit Shell-Metazeichen kritisch:
# ucr search --brief --value '[*?$`]'
ldap/connection/check: */3 * * * *
ldap/policy/cron: 5 * * * *
mrtg/cron: */15 * * * *

Siehe Bug #8417 Comment 7 für weitere Informationen.
Ggf. ist Bug #17269 auch auf diese Problematik zurückzuführen.

Alle Skripte sollten korrigiert werden und ggf. ein ucs-test-Test oder ucs-lint-Check eingebaut werden, der fehlerhaften Verwendung anprangert.
Comment 1 Philipp Hahn univentionstaff 2010-05-20 11:30:13 CEST
Derzeit werden 801 Fälle gefunden von:
$ find \( -name .git -o -name .svn -o -name .pc -o -name .metadata \) -prune -o -type f -exec grep -E -l 'eval[[:space:]]+(\$\(|`)(/usr/sbin/)?(ucr|univention-config-registry|univention-baseconfig)[[:space:]]+.*shell.*[)`]' {} + | wc -l
Comment 2 Philipp Hahn univentionstaff 2011-10-07 08:44:38 CEST
*** Bug 23985 has been marked as a duplicate of this bug. ***
Comment 3 Stefan Gohmann univentionstaff 2011-10-07 08:59:29 CEST
Gibt es dazu eine ucslint Prüfung?
Comment 4 Sönke Schwardt-Krummrich univentionstaff 2011-10-07 09:07:19 CEST
Jein. ucslint prüft derzeit nur im postinst (0006-7) und in Joinskripten (0001-10) auf folgende Konstrukte:

eval $(ucr shell)
eval `ucr shell`

aber nicht auf

eval $(ucr shell FOOBAR)
eval `ucr shell FOOBAR`
Comment 5 Stefan Gohmann univentionstaff 2011-10-07 09:29:21 CEST
Wir sollten zu 3.1 die uclint Prüfung implementieren und dann die Pakete entsprechend anpassen.
Comment 6 Philipp Hahn univentionstaff 2011-10-22 09:01:45 CEST
Ein Teil ist in svn28113 geschehen.
Comment 7 Lukas Walter univentionstaff 2012-07-31 10:37:26 CEST
Ich konnte nurnoch einen weiteren Aufruf von eval $(ucr shell) ohne quotes finden (in univention-samba/addmachine.sh) - den habe ich korrigiert.

Darüber hinaus habe ich ucslint wie gewünscht um einen Test erweitert, der alle Dateien eines Pakets auf eval $(ucr shell) Aufrufe ohne quotes prüft.
Die Prüfung aus dem Test des postinst fällt dafür weg.


univention-samba (7.0.1-10) unstable; urgency=low

  * added quotes to eval $(ucr shell) call in addmachine.sh
   (Bug #18443)

rev34506


ucslint (3.0.1-1) unstable; urgency=low

  * added a test for unquoted calls of eval $(ucr shell)
  * removed testing for unquoted calls of eval $(ucr shell)
    from 0006 and 0001
    (Bug #18443)

rev34505
Comment 8 Philipp Hahn univentionstaff 2012-07-31 10:50:13 CEST
Bezüglich svn34505:

ucslint/0001-CheckJoinScript.py
- ... '0001-10': ... 'join script contains "eval $(ucr shell)" without proper quoting' ],
- ... '0001-11': ... 'join script contains lines with unquoted $@' ],
- ... '0001-12': ... 'join script contains more than one line with VERSION=  statement' ],
- ... '0001-13': ... 'join script does not include "joinscripthelper.lib"' ],
- ... '0001-14': ... 'join script does not call "joinscript_init"' ],
- ... '0001-15': ... 'join script does not call "joinscript_save_current_version"' ],
+ ... '0001-10': ... 'join script contains lines with unquoted $@' ],
+ ... '0001-11': ... 'join script contains more than one line with VERSION=  statement' ],
+ ... '0001-12': ... 'join script does not include "joinscripthelper.lib"' ],
+ ... '0001-13': ... 'join script does not call "joinscript_init"' ],
+ ... '0001-14': ... 'join script does not call "joinscript_save_current_version"' ],

Die IDs müssen stabil bleiben und dürfen NICHT neu durchnummeriert werden, weil
diese auch in den ganzen debian/ucslint.overrides Dateien verwendet werden.
Bitte zumindest das Rückgängig machen und 0001-10 irgendwie als "Unused, moved
to 0017-*" markieren.

ucslint/0017-Shell.py
+               for root, dirs, files in os.walk('.'):
Hier bitte den FilteredDirWalkGenerator() aus ucslint/base.py benutzen, weil
der korrekt mit den ganzen Dateien umgeht, die ignoriert werden sollen.
Comment 9 Lukas Walter univentionstaff 2012-07-31 12:00:06 CEST
(In reply to comment #8)
> Bezüglich svn34505:
> 
> ucslint/0001-CheckJoinScript.py
> - ... '0001-10': ... 'join script contains "eval $(ucr shell)" without proper
> quoting' ],
> - ... '0001-11': ... 'join script contains lines with unquoted $@' ],
> - ... '0001-12': ... 'join script contains more than one line with VERSION= 
> statement' ],
> - ... '0001-13': ... 'join script does not include "joinscripthelper.lib"' ],
> - ... '0001-14': ... 'join script does not call "joinscript_init"' ],
> - ... '0001-15': ... 'join script does not call
> "joinscript_save_current_version"' ],
> + ... '0001-10': ... 'join script contains lines with unquoted $@' ],
> + ... '0001-11': ... 'join script contains more than one line with VERSION= 
> statement' ],
> + ... '0001-12': ... 'join script does not include "joinscripthelper.lib"' ],
> + ... '0001-13': ... 'join script does not call "joinscript_init"' ],
> + ... '0001-14': ... 'join script does not call
> "joinscript_save_current_version"' ],
> 
> Die IDs müssen stabil bleiben und dürfen NICHT neu durchnummeriert werden, weil
> diese auch in den ganzen debian/ucslint.overrides Dateien verwendet werden.
> Bitte zumindest das Rückgängig machen und 0001-10 irgendwie als "Unused, moved
> to 0017-*" markieren.
> 
> ucslint/0017-Shell.py
> +               for root, dirs, files in os.walk('.'):
> Hier bitte den FilteredDirWalkGenerator() aus ucslint/base.py benutzen, weil
> der korrekt mit den ganzen Dateien umgeht, die ignoriert werden sollen.

Wurde soweit angepasst.
rev34511
Comment 10 Sönke Schwardt-Krummrich univentionstaff 2012-09-14 14:34:51 CEST
OK: ucslint prüft jetzt auf alle Varianten von eval $(ucr shell)
OK: ucslint verwendet DirWalkGenerator
OK: ucslint Testframework
OK: keine weiteren Vorkommnisse in UCS 3.1 gefunden
OK: Changelogeintrag
Comment 11 Stefan Gohmann univentionstaff 2012-12-12 21:09:18 CET
UCS 3.1-0 has been released: 
 http://forum.univention.de/viewtopic.php?f=54&t=2125

If this error occurs again, please use "Clone This Bug".