On Sun, Jan 28, 2007 at 11:26:49PM +0100, Brice Goglin wrote:
> Jeremie Koenig wrote:
[...]
> > I also noticed two similar problems with the generation of thumbnails
> > and scaled images (llgal.in, lines 597 and 690):
[...]
> This case is supposed to be protected by double-quotes. Did you actually
> see a problem occur there? Or did you just find a possible problem by
> looking at the code?
I found it looking at the code, but the double quotes won't protect
double quotes, dollar signs or backquotes within filenames from being
interpreted by the shell. Here is an example:
[EMAIL PROTECTED]:~/test$ ls -l
total 532
-rw-r--r-- 1 jk jk 535037 2007-01-28 23:34 ";echo Hello, World.;.jpg
-rw-r--r-- 1 jk jk 705 2007-01-28 23:40 index.html
-rw-r--r-- 1 jk jk 0 2007-01-28 23:47 typescript
[EMAIL PROTECTED]:~/test$ llgal
Listing entries in . : 100.00%
Preparing entries: 100.00%
!! Failed to create '";echo Hello, World.;.jpg' thumbnail.
!! convert: missing an image filename `'.
!! Hello, World.
!! sh: .jpg .llgal/thumb_;echo Hello, World.;.jpg: Aucun fichier ou répertoire
de ce type
Found 0 entries in current directory
Using '/usr/share/llgal/slidetemplate.html' as HTML slide template.
Creating individual slides: 100.00%
Using '/usr/share/llgal/indextemplate.html' as HTML index template.
Creating the index.html file: 100.00%
Found llgal.css in .llgal/, using it.
> > + my @cmdline = map {
> > + s/<IN>/$real_filename/g ;
> > + s/<OUT>/$real_thumb_filename/g ;
> > + $_ ;
> > + } split (' ', $opts->{thumbnail_create_command}) ;
> I don't think this split will work if the image filename contains
> spaces.
llgal with the patch applied processes the above proof-of-concept
directory all right. Note that only $opts->{thumbnail_create_command} is
passed through split, the image filenames are substituted once this is
done, within the resulting list. Then 'exec' within 'system_with_output'
uses each element as a disctinct argument, and does the syscall
directly (ie., without using a shell).
Apart from the slight interface change this implies, the patch should be
okay (the fixed llgal is processing my collection successfully right
now, which has all kinds of unhealthily named files :-).
Maybe drop a note in the manpage documenting the fact that llgal no
longer uses /bin/sh to run the given thumbnail_create_command, but I
wouldn't expect many people (or anyone) to be relying on this anyway.
