[Kde-imaging] Fwd: Re: nasty bug in libkexif

[Tom Albers]

Indeed, a new release of libkexif is needed. I guess Gilles has the most time to do this. 

Toma

At Tuesday 17 October 2006 23:08, you wrote:
> Hi,
> this effect is probably not so present in linux,
> but it can happen and make people loosing data,
> as reported by Marc in OpenBSD.
> 
> I believe we should plan another libkexif with this
> fix. Unfortunately I can't do it before next
> weekend I believe.
> 
> If there are no volonteers, but you want to patch 
> distro packages in attach a patch is present.
> 
> Regards,
>    Angelo  
> 
> ----------  Messaggio inoltrato  ----------
> 
> Subject: Re: nasty bug in libkexif
> Date: 22:45, martedì 17 ottobre 2006
> From: Marc Espie <espie at nerim.net>
> To: Angelo Naselli <anaselli at linux.it>
> Cc: Gilles Caulier <caulier.gilles at free.fr>
> 
> On Tue, Oct 17, 2006 at 10:26:44PM +0200, Angelo Naselli wrote:
> > Alle 21:53, martedì 17 ottobre 2006, Marc Espie ha scritto:
> > > On Tue, Oct 17, 2006 at 09:33:51PM +0200, Gilles Caulier wrote:
> > > > Thanks Marcs for this report.
> > > > 
> > > > I forward your mail to Angelo to include this fix in TODO list.
> > > > 
> > > > Note : digikam and kipi-plugins from KDE svn trunk do use libkexif
> naymore. We 
> > > > use Exiv2 library instead (more powerfull)
> > > > 
> > > > This is want mean than for next final release of digiKam (0.9.0) and 
> > > > kipi-plugins (0.1.3), your bug will disapears...
> > > > 
> > > > Angelo, fell free to fix this little bug in svn. Thanks in advance
> > > 
> > > I investigated a bit more, my fopen(3) library explicitly says:
> > > 
> > >      Reads and writes cannot be arbitrarily intermixed on read/write
> streams.
> > >      ANSI C requires that a file positioning function intervene between
> output
> > >      and input, unless an input operation encounters end-of-file.
> > Well as far as I can read into the same manual under linux:
> >  
> >        Reads and writes may be intermixed on read/write streams in any  order.
> >        Note  that  ANSI  C requires that a file positioning function intervene
> >        between output and input, unless an input operation encounters  end-of-
> >        file.   (If this condition is not met, then a read is allowed to return
> >        the result of writes other than the most recent.)  Therefore it is good
> >        practice (and indeed sometimes necessary under Linux) to put an fseek()
> >        or fgetpos() operation between write and  read  operations  on  such  a
> >        stream.   This operation may be an apparent no-op (as in fseek(..., 0L,
> >        SEEK_CUR) called for its synchronizing side effect.
> > 
> > The problem should be present also here. 
> 
> This manual page says the linux implementation will most often allow you
> to intermix read and write without effect.
> 
> > > And Qt's QDataStream, using a QFile as device in non-raw mode, does mix
> > > fread() and fwrite() without any kind of seek()'ing, so indeed, this will
> > > not work on some C implementations, at least.
> > Frankly speaking I don't know qt implementation and not so well kexif either.
> > So Gilles what do you suggest? And which is the bad thing that can be happen
> > with the implementation we have by now?
> > I mean we can loose data? Security issue? 
> 
> yes, you can lose data. I found this issue by tracking a bug in digikam 
> where I was ending up in corrupted images on OpenBSD.
> 
> I managed to get the problem down to this code:
> 
> #include <libkexif/kexifdata.h>
> #include <libkexif/kexifutils.h>
> #include <stdlib.h>
> 
> int
> main(int argc, char *argv[])
> {
>         if (argc < 2) {
>                 exit(1);
>         }
>         KExifUtils::writeOrientation(argv[1], KExifData::NORMAL);
>         exit(0);
> }
> 
> under OpenBSD, this code reliably corrupts jpeg images with exif data.
> 
> In one image, the orientation marker was near the beginning of the
> file, at offset ~100, but libkexif did overwrite it at offset 16384 instead,
> erasing two unrelated bytes (the size of the file buffer, coincidentally).
> 
> The following patch makes sure stuff get synch'ed properly. I've tested that
> it fixes the issue.
> 
> --- libkexif/kexifutils.cpp.orig	Tue Sep  6 19:30:06 2005
> +++ libkexif/kexifutils.cpp	Tue Oct 17 21:32:35 2006
> @@ -38,6 +38,11 @@ extern "C"
>  #include "kexifutils.h"
>  #include "kexifdata.h"
>  
> +static void streamsync(QDataStream &s)
> +{
> +	s.device()->at(s.device()->at());
> +}
> +
>  bool KExifUtils::writeOrientation(const QString& filename,
> KExifData::ImageOrientation orientation)
>  {
>     QString str = "";
> @@ -217,6 +222,7 @@ bool KExifUtils::writeFile(const QString
>            position++;
>         }
>  
> +       streamsync(stream);
>         // write character code ASCII into offset position
>         stream << 0x49435341;
>         stream << 0x00000049;
> @@ -260,6 +266,7 @@ bool KExifUtils::writeFile(const QString
>                if(i > 8  && buf[i]==0x01 && buf[i-1]==0x00 && buf[i-2]==0x00 &&
> buf[i-3]==0x00 
>                        && buf[i-4]==0x03 && buf[i-5]==0x00 && buf[i-6]==0x12 &&
> buf[i-7]==0x01)
>                {
> +	          streamsync(stream);
>                    stream << (Q_UINT8)0x00;
>                    stream << (Q_UINT8)orientation;
>  
> @@ -270,6 +277,7 @@ bool KExifUtils::writeFile(const QString
>                if(i > 8 && buf[i]==0x00 && buf[i-1]==0x00 && buf[i-2]==0x00 &&
> buf[i-3]==0x01 
>                       && buf[i-4]==0x00 && buf[i-5]==0x03 && buf[i-6]==0x01 &&
> buf[i-7]==0x12)
>                {
> +	          streamsync(stream);
>                    stream << (Q_UINT8)orientation;
>                    stream << (Q_UINT8)0x00;
>  
> 
> 
> -------------------------------------------------------