[PATCH] preliminary volume formatting support

[Christopher Santero]

On 1/20/06, David Zeuthen <david at fubar.dk> wrote:
> > +# For now, only root can format
> > +if [ $HAL_METHOD_INVOKED_BY_UID != 0 ] ; then
> > +     echo "Only root is allowed to format volumes using this tool."
> > +     exit 1
> > +fi
>
> You need to pass the exception name first, e.g.
>
>         echo "org.freedesktop.Hal.Device.Volume.Unstable.PermissionDenied" >&2
>         echo "Only root is allowed to format volumes using this tool."
>         exit 1
>
> instead. This comment applies other places in your script too.
>

Added the exceptions.

> Also here
>
> > +# Read parameters
> > +read FS_TYPE
> > +read LABEL
>
> you really really want to guard against a malicious caller passing
> `/path/to/some/binary` as you expand $FS_TYPE below. See for example
> tools/hal-system-storage-mount on how we guard against this - yes, both
> Kay and I did the same mistake with the mounting scripts..

Isn't this taken care of by my case statement? If the input FS_TYPE
string is not one of the supported filesystems, it jumps to *) and
throws the InvalidFilesystemType exception. Then again my bash mojo is
not yet strong. =)

Reading LABEL could be problematic though, since an attacker could
manipulate the command line that way. I added quotes, should this fix
that problem?

read LABEL
LABEL="\"$LABEL\""

>
> > +             FORMAT_CMD="/sbin/mkfs.ext2 $HAL_PROP_BLOCK_DEVICE"
>
> Suggest to just use mkfs.ext2 as we control $PATH when launching hald;
> this makes it easier if some distribution puts mkfs.ext2 in strange
> places. Oh, and this comment applies other places in your script too.

I cut out the absolute paths, and since the commands I needed are
sbins, I did this:

PATH="/usr/local/sbin:/usr/sbin:/sbin:$PATH"

New patch attached. I also uncommented the check to see if the volume
was mounted. I had been having other weird problems too, until I
noticed the little line in the 0.5.6 NEWS that says kernel 2.6.15 is
required. Meeting dependencies solves lots of things. =)

Chris
-------------- next part --------------
A non-text attachment was scrubbed...
Name: hal-volume-format-0.2.patch
Type: text/x-patch
Size: 5189 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/hal/attachments/20060121/7373af8b/hal-volume-format-0.2.bin