Hi,

I've investigated Debian Bug #612417[1]. The problems seems to
be /usr/lib/grub/grub-mkconfig_lib. It can't handle arguments with
whitespace in it because some variables are not properly quoted.

I've attached three patches:

add-quoting-small.patch:
        This patch fixes the bug with a minimal set of changes.

add-quoting-big.patch:
        This patch fixes the bug and does also add proper quoting to 
        the rest of the file if it is safe to do so. This means that
        lines like these have NOT been changed:
        
                for foo in ${list}; do bar; done

        I also didn't change lines like this,

                ${grub_probe} --foo --bar

        because ${grub_probe} might contain additional parameters.

add-quoting-really-big.patch:
        This patch is identical to the previous one with the exception 
        of the ${grub_probe} case which is now also quoted.

In general it seems to be a good a idea to add proper quoting to
grub-mkconfig_lib for additional robustness. I've carefully written and
tested all of the attached patches and everything works just fine for
me. However, I can not guarantee that

        a) I didn't miss any unquoted variables.
        b) I didn't break some special use case.
        c) I didn't break someones clever hack due to a lack of 
           understanding.

Please review the attached patches and apply the best one. If there are
any problems with the patches, don't hesitate to tell me so!

Best regards

Alexander Kurtz

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=612417
--- grub-mkconfig_lib.old	2011-02-08 12:13:51.524477788 +0100
+++ grub-mkconfig_lib	2011-02-08 12:16:33.727467298 +0100
@@ -44,20 +44,20 @@
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  ${grub_mkrelpath} "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if ${grub_probe} -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 
--- grub-mkconfig_lib.old	2011-02-08 12:13:51.524477788 +0100
+++ grub-mkconfig_lib	2011-02-08 12:49:33.222340825 +0100
@@ -16,19 +16,19 @@
 
 transform="s,x,x,"
 
-prefix=/usr
-exec_prefix=${prefix}
-datarootdir=${prefix}/share
-datadir=${datarootdir}
-bindir=${exec_prefix}/bin
-sbindir=${exec_prefix}/sbin
-pkgdatadir=${datadir}/`echo grub | sed "${transform}"`
+prefix="/usr"
+exec_prefix="${prefix}"
+datarootdir="${prefix}/share"
+datadir="${datarootdir}"
+bindir="${exec_prefix}/bin"
+sbindir="${exec_prefix}/sbin"
+pkgdatadir="${datadir}/`echo grub | sed "${transform}"`"
 
-if test "x$grub_probe" = x; then
-  grub_probe=${sbindir}/`echo grub-probe | sed ${transform}`
+if test "x${grub_probe}" = "x"; then
+  grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`"
 fi
-if test "x$grub_mkrelpath" = x; then
-  grub_mkrelpath=${bindir}/`echo grub-mkrelpath | sed ${transform}`
+if test "x${grub_mkrelpath}" = "x"; then
+  grub_mkrelpath="${bindir}/`echo grub-mkrelpath | sed "${transform}"`"
 fi
 
 if $(which gettext >/dev/null 2>/dev/null) ; then
@@ -39,25 +39,25 @@
 
 grub_warn ()
 {
-  echo "Warning: $@" >&2
+  echo "Warning: ${@}" >&2
 }
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  ${grub_mkrelpath} "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if ${grub_probe} -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 
@@ -66,24 +66,24 @@
 
 convert_system_path_to_grub_path ()
 {
-  path=$1
+  path="${1}"
 
   grub_warn "convert_system_path_to_grub_path() is deprecated.  Use prepare_grub_to_access_device() instead."
 
   # abort if GRUB can't access the path
-  if is_path_readable_by_grub ${path} ; then : ; else
+  if is_path_readable_by_grub "${path}" ; then : ; else
     return 1
   fi
 
-  if drive=`${grub_probe} -t drive $path` ; then : ; else
+  if drive="`${grub_probe} -t drive "${path}"`" ; then : ; else
     return 1
   fi
 
-  if relative_path=`make_system_path_relative_to_its_root $path` ; then : ; else
+  if relative_path="`make_system_path_relative_to_its_root "${path}"`" ; then : ; else
     return 1
   fi
 
-  echo ${drive}${relative_path}
+  echo "${drive}${relative_path}"
 }
 
 save_default_entry ()
@@ -97,11 +97,11 @@
 
 prepare_grub_to_access_device ()
 {
-  device=$1
+  device="${1}"
 
-  if dmsetup status $device 2>/dev/null | grep -q 'crypt[[:space:]]$'; then
+  if dmsetup status "${device}" 2>/dev/null | grep -q 'crypt[[:space:]]$'; then
     grub_warn \
-      "$device is a crypto device, which GRUB cannot read directly.  Some" \
+      "${device} is a crypto device, which GRUB cannot read directly.  Some" \
       "necessary modules may be missing from /boot/grub/grub.cfg.  You may" \
       "need to write an /etc/grub.d/01_modules script to load them.  See" \
       "http://bugs.debian.org/542165 for details."
@@ -109,12 +109,12 @@
   fi
 
   # Abstraction modules aren't auto-loaded.
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`${grub_probe} --device "${device}" --target=abstraction`"
   for module in ${abstraction} ; do
     echo "insmod ${module}"
   done
 
-  partmap="`${grub_probe} --device ${device} --target=partmap`"
+  partmap="`${grub_probe} --device "${device}" --target=partmap`"
   for module in ${partmap} ; do
     case "${module}" in
       netbsd | openbsd)
@@ -124,23 +124,23 @@
     esac
   done
 
-  fs="`${grub_probe} --device ${device} --target=fs`"
+  fs="`${grub_probe} --device "${device}" --target=fs`"
   for module in ${fs} ; do
     echo "insmod ${module}"
   done
 
   # If there's a filesystem UUID that GRUB is capable of identifying, use it;
   # otherwise set root as per value in device.map.
-  echo "set root='`${grub_probe} --device ${device} --target=drive`'"
-  if fs_uuid="`${grub_probe} --device ${device} --target=fs_uuid 2> /dev/null`" ; then
+  echo "set root='`${grub_probe} --device "${device}" --target=drive`'"
+  if fs_uuid="`${grub_probe} --device "${device}" --target=fs_uuid 2> /dev/null`" ; then
     echo "search --no-floppy --fs-uuid --set=root ${fs_uuid}"
   fi
 }
 
 grub_file_is_not_garbage ()
 {
-  if test -f "$1" ; then
-    case "$1" in
+  if test -f "${1}" ; then
+    case "${1}" in
       *.dpkg-*) return 1 ;; # debian dpkg
       README*)  return 1 ;; # documentation
     esac
@@ -152,21 +152,21 @@
 
 version_test_numeric ()
 {
-  local a=$1
-  local cmp=$2
-  local b=$3
-  if [ "$a" = "$b" ] ; then
-    case $cmp in
+  local a="${1}"
+  local cmp="${2}"
+  local b="${3}"
+  if [ "${a}" = "${b}" ] ; then
+    case "${cmp}" in
       ge|eq|le) return 0 ;;
       gt|lt) return 1 ;;
     esac
   fi
-  if [ "$cmp" = "lt" ] ; then
-    c=$a
-    a=$b
-    b=$c
+  if [ "${cmp}" = "lt" ] ; then
+    c="${a}"
+    a="${b}"
+    b="${c}"
   fi
-  if (echo $a ; echo $b) | sort -n | head -n 1 | grep -qx $b ; then
+  if (echo "${a}" ; echo "${b}") | sort -n | head -n 1 | grep -qx "${b}" ; then
     return 0
   else
     return 1
@@ -176,54 +176,54 @@
 version_test_gt ()
 {
   local sedexp="s/[^-]*-//;s/[._-]\(pre\|rc\|test\|git\|old\|trunk\)/~\1/g"
-  local a=`echo $1 | sed -e "$sedexp"`
-  local b=`echo $2 | sed -e "$sedexp"`
+  local a="`echo "${1}" | sed -e "${sedexp}"`"
+  local b="`echo "${2}" | sed -e "${sedexp}"`"
   local cmp=gt
-  if [ "x$b" = "x" ] ; then
+  if [ "x${b}" = "x" ] ; then
     return 0
   fi
-  case $a:$b in
+  case "${a}:${b}" in
     *.old:*.old) ;;
-    *.old:*) a=`echo -n $a | sed -e s/\.old$//` ; cmp=gt ;;
-    *:*.old) b=`echo -n $b | sed -e s/\.old$//` ; cmp=ge ;;
+    *.old:*) a="`echo -n "${a}" | sed -e 's/\.old$//'`" ; cmp=gt ;;
+    *:*.old) b="`echo -n "${b}" | sed -e 's/\.old$//'`" ; cmp=ge ;;
   esac
-  dpkg --compare-versions "$a" $cmp "$b"
-  return $?
+  dpkg --compare-versions "${a}" "${cmp}" "${b}"
+  return "${?}"
 }
 
 version_find_latest ()
 {
   local a=""
-  for i in $@ ; do
-    if version_test_gt "$i" "$a" ; then
-      a="$i"
+  for i in "${@}" ; do
+    if version_test_gt "${i}" "${a}" ; then
+      a="${i}"
     fi
   done
-  echo "$a"
+  echo "${a}"
 }
 
 # One layer of quotation is eaten by "", the second by sed, and the third by
 # printf; so this turns ' into \'.  Note that you must use the output of
 # this function in a printf format string.
 gettext_quoted () {
-  $gettext "$@" | sed "s/'/'\\\\\\\\''/g"
+  ${gettext} "${@}" | sed "s/'/'\\\\\\\\''/g"
 }
 
 # Run the first argument through gettext_quoted, and then pass that and all
 # remaining arguments to printf.  This is a useful abbreviation and tends to
 # be easier to type.
 gettext_printf () {
-  local format="$1"
+  local format="${1}"
   shift
-  printf "$(gettext_quoted "$format")" "$@"
+  printf "`gettext_quoted "${format}"`" "${@}"
 }
 
 uses_abstraction () {
-  device=$1
+  device="${1}"
 
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`${grub_probe} --device "${device}" --target=abstraction`"
   for module in ${abstraction}; do
-    if test "x${module}" = "x$2"; then
+    if test "x${module}" = "x${2}"; then
       return 0
     fi
   done
--- grub-mkconfig_lib.old	2011-02-08 12:13:51.524477788 +0100
+++ grub-mkconfig_lib	2011-02-08 12:56:24.250350460 +0100
@@ -16,19 +16,19 @@
 
 transform="s,x,x,"
 
-prefix=/usr
-exec_prefix=${prefix}
-datarootdir=${prefix}/share
-datadir=${datarootdir}
-bindir=${exec_prefix}/bin
-sbindir=${exec_prefix}/sbin
-pkgdatadir=${datadir}/`echo grub | sed "${transform}"`
+prefix="/usr"
+exec_prefix="${prefix}"
+datarootdir="${prefix}/share"
+datadir="${datarootdir}"
+bindir="${exec_prefix}/bin"
+sbindir="${exec_prefix}/sbin"
+pkgdatadir="${datadir}/`echo grub | sed "${transform}"`"
 
-if test "x$grub_probe" = x; then
-  grub_probe=${sbindir}/`echo grub-probe | sed ${transform}`
+if test "x${grub_probe}" = "x"; then
+  grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`"
 fi
-if test "x$grub_mkrelpath" = x; then
-  grub_mkrelpath=${bindir}/`echo grub-mkrelpath | sed ${transform}`
+if test "x${grub_mkrelpath}" = "x"; then
+  grub_mkrelpath="${bindir}/`echo grub-mkrelpath | sed "${transform}"`"
 fi
 
 if $(which gettext >/dev/null 2>/dev/null) ; then
@@ -39,25 +39,25 @@
 
 grub_warn ()
 {
-  echo "Warning: $@" >&2
+  echo "Warning: ${@}" >&2
 }
 
 make_system_path_relative_to_its_root ()
 {
-  ${grub_mkrelpath} $1
+  "${grub_mkrelpath}" "${1}"
 }
 
 is_path_readable_by_grub ()
 {
-  path=$1
+  path="${1}"
 
   # abort if path doesn't exist
-  if test -e $path ; then : ;else
+  if test -e "${path}" ; then : ;else
     return 1
   fi
 
   # abort if file is in a filesystem we can't read
-  if ${grub_probe} -t fs $path > /dev/null 2>&1 ; then : ; else
+  if "${grub_probe}" -t fs "${path}" > /dev/null 2>&1 ; then : ; else
     return 1
   fi
 
@@ -66,24 +66,24 @@
 
 convert_system_path_to_grub_path ()
 {
-  path=$1
+  path="${1}"
 
   grub_warn "convert_system_path_to_grub_path() is deprecated.  Use prepare_grub_to_access_device() instead."
 
   # abort if GRUB can't access the path
-  if is_path_readable_by_grub ${path} ; then : ; else
+  if is_path_readable_by_grub "${path}" ; then : ; else
     return 1
   fi
 
-  if drive=`${grub_probe} -t drive $path` ; then : ; else
+  if drive="`"${grub_probe}" -t drive "${path}"`" ; then : ; else
     return 1
   fi
 
-  if relative_path=`make_system_path_relative_to_its_root $path` ; then : ; else
+  if relative_path="`make_system_path_relative_to_its_root "${path}"`" ; then : ; else
     return 1
   fi
 
-  echo ${drive}${relative_path}
+  echo "${drive}${relative_path}"
 }
 
 save_default_entry ()
@@ -97,11 +97,11 @@
 
 prepare_grub_to_access_device ()
 {
-  device=$1
+  device="${1}"
 
-  if dmsetup status $device 2>/dev/null | grep -q 'crypt[[:space:]]$'; then
+  if dmsetup status "${device}" 2>/dev/null | grep -q 'crypt[[:space:]]$'; then
     grub_warn \
-      "$device is a crypto device, which GRUB cannot read directly.  Some" \
+      "${device} is a crypto device, which GRUB cannot read directly.  Some" \
       "necessary modules may be missing from /boot/grub/grub.cfg.  You may" \
       "need to write an /etc/grub.d/01_modules script to load them.  See" \
       "http://bugs.debian.org/542165 for details."
@@ -109,12 +109,12 @@
   fi
 
   # Abstraction modules aren't auto-loaded.
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`"${grub_probe}" --device "${device}" --target=abstraction`"
   for module in ${abstraction} ; do
     echo "insmod ${module}"
   done
 
-  partmap="`${grub_probe} --device ${device} --target=partmap`"
+  partmap="`"${grub_probe}" --device "${device}" --target=partmap`"
   for module in ${partmap} ; do
     case "${module}" in
       netbsd | openbsd)
@@ -124,23 +124,23 @@
     esac
   done
 
-  fs="`${grub_probe} --device ${device} --target=fs`"
+  fs="`"${grub_probe}" --device "${device}" --target=fs`"
   for module in ${fs} ; do
     echo "insmod ${module}"
   done
 
   # If there's a filesystem UUID that GRUB is capable of identifying, use it;
   # otherwise set root as per value in device.map.
-  echo "set root='`${grub_probe} --device ${device} --target=drive`'"
-  if fs_uuid="`${grub_probe} --device ${device} --target=fs_uuid 2> /dev/null`" ; then
+  echo "set root='`"${grub_probe}" --device "${device}" --target=drive`'"
+  if fs_uuid="`"${grub_probe}" --device "${device}" --target=fs_uuid 2> /dev/null`" ; then
     echo "search --no-floppy --fs-uuid --set=root ${fs_uuid}"
   fi
 }
 
 grub_file_is_not_garbage ()
 {
-  if test -f "$1" ; then
-    case "$1" in
+  if test -f "${1}" ; then
+    case "${1}" in
       *.dpkg-*) return 1 ;; # debian dpkg
       README*)  return 1 ;; # documentation
     esac
@@ -152,21 +152,21 @@
 
 version_test_numeric ()
 {
-  local a=$1
-  local cmp=$2
-  local b=$3
-  if [ "$a" = "$b" ] ; then
-    case $cmp in
+  local a="${1}"
+  local cmp="${2}"
+  local b="${3}"
+  if [ "${a}" = "${b}" ] ; then
+    case "${cmp}" in
       ge|eq|le) return 0 ;;
       gt|lt) return 1 ;;
     esac
   fi
-  if [ "$cmp" = "lt" ] ; then
-    c=$a
-    a=$b
-    b=$c
+  if [ "${cmp}" = "lt" ] ; then
+    c="${a}"
+    a="${b}"
+    b="${c}"
   fi
-  if (echo $a ; echo $b) | sort -n | head -n 1 | grep -qx $b ; then
+  if (echo "${a}" ; echo "${b}") | sort -n | head -n 1 | grep -qx "${b}" ; then
     return 0
   else
     return 1
@@ -176,54 +176,54 @@
 version_test_gt ()
 {
   local sedexp="s/[^-]*-//;s/[._-]\(pre\|rc\|test\|git\|old\|trunk\)/~\1/g"
-  local a=`echo $1 | sed -e "$sedexp"`
-  local b=`echo $2 | sed -e "$sedexp"`
+  local a="`echo "${1}" | sed -e "${sedexp}"`"
+  local b="`echo "${2}" | sed -e "${sedexp}"`"
   local cmp=gt
-  if [ "x$b" = "x" ] ; then
+  if [ "x${b}" = "x" ] ; then
     return 0
   fi
-  case $a:$b in
+  case "${a}:${b}" in
     *.old:*.old) ;;
-    *.old:*) a=`echo -n $a | sed -e s/\.old$//` ; cmp=gt ;;
-    *:*.old) b=`echo -n $b | sed -e s/\.old$//` ; cmp=ge ;;
+    *.old:*) a="`echo -n "${a}" | sed -e 's/\.old$//'`" ; cmp=gt ;;
+    *:*.old) b="`echo -n "${b}" | sed -e 's/\.old$//'`" ; cmp=ge ;;
   esac
-  dpkg --compare-versions "$a" $cmp "$b"
-  return $?
+  dpkg --compare-versions "${a}" "${cmp}" "${b}"
+  return "${?}"
 }
 
 version_find_latest ()
 {
   local a=""
-  for i in $@ ; do
-    if version_test_gt "$i" "$a" ; then
-      a="$i"
+  for i in "${@}" ; do
+    if version_test_gt "${i}" "${a}" ; then
+      a="${i}"
     fi
   done
-  echo "$a"
+  echo "${a}"
 }
 
 # One layer of quotation is eaten by "", the second by sed, and the third by
 # printf; so this turns ' into \'.  Note that you must use the output of
 # this function in a printf format string.
 gettext_quoted () {
-  $gettext "$@" | sed "s/'/'\\\\\\\\''/g"
+  "${gettext}" "${@}" | sed "s/'/'\\\\\\\\''/g"
 }
 
 # Run the first argument through gettext_quoted, and then pass that and all
 # remaining arguments to printf.  This is a useful abbreviation and tends to
 # be easier to type.
 gettext_printf () {
-  local format="$1"
+  local format="${1}"
   shift
-  printf "$(gettext_quoted "$format")" "$@"
+  printf "`gettext_quoted "${format}"`" "${@}"
 }
 
 uses_abstraction () {
-  device=$1
+  device="${1}"
 
-  abstraction="`${grub_probe} --device ${device} --target=abstraction`"
+  abstraction="`"${grub_probe}" --device "${device}" --target=abstraction`"
   for module in ${abstraction}; do
-    if test "x${module}" = "x$2"; then
+    if test "x${module}" = "x${2}"; then
       return 0
     fi
   done

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Grub-devel mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to