After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 690524 - Fix reading from OLE files not multiple of block size
Fix reading from OLE files not multiple of block size
Status: RESOLVED FIXED
Product: libgsf
Classification: Core
Component: MS OLE2 & Properties
unspecified
Other All
: Normal normal
: ---
Assigned To: Stepan Kasal
Jody Goldberg
Depends on:
Blocks:
 
 
Reported: 2012-12-19 20:42 UTC by Marc-Andre Lureau
Modified: 2013-02-23 02:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
ole: update OLECF specification link (831 bytes, patch)
2012-12-19 20:42 UTC, Marc-Andre Lureau
committed Details | Review
ole: factour out seeking to block (2.40 KB, patch)
2012-12-19 20:42 UTC, Marc-Andre Lureau
committed Details | Review
ole: only read amount needed from block (1.04 KB, patch)
2012-12-19 20:42 UTC, Marc-Andre Lureau
none Details | Review
ole: round-up the number of blocks (1.17 KB, patch)
2012-12-19 20:42 UTC, Marc-Andre Lureau
none Details | Review
ole: fix TODO to avoid extra copy when reading (1.14 KB, patch)
2012-12-19 20:42 UTC, Marc-Andre Lureau
none Details | Review

Description Marc-Andre Lureau 2012-12-19 20:42:30 UTC
Some files are not multiple of block size in size. Reading the last
block currently fails.

A few misc. patches are also added in the series as bonus.
Comment 1 Marc-Andre Lureau 2012-12-19 20:42:32 UTC
Created attachment 231928 [details] [review]
ole: update OLECF specification link
Comment 2 Marc-Andre Lureau 2012-12-19 20:42:36 UTC
Created attachment 231929 [details] [review]
ole: factour out seeking to block
Comment 3 Marc-Andre Lureau 2012-12-19 20:42:39 UTC
Created attachment 231930 [details] [review]
ole: only read amount needed from block

Some OLE files are not multiple of block size, and may file to read
the last block when its size is smaller.
Comment 4 Marc-Andre Lureau 2012-12-19 20:42:43 UTC
Created attachment 231931 [details] [review]
ole: round-up the number of blocks

Some files are not multiple of block size, we may still want to seek to the last block
Comment 5 Marc-Andre Lureau 2012-12-19 20:42:47 UTC
Created attachment 231932 [details] [review]
ole: fix TODO to avoid extra copy when reading
Comment 6 Morten Welinder 2012-12-21 00:24:37 UTC
I have committed the first two parts.

What generates such files with a partial final block?  Can you attach
a sample, please?
Comment 7 Marc-Andre Lureau 2012-12-21 00:35:05 UTC
It's a bit large to copy here:

It came from the GStreamer merge modules. I tried to read the base-system-2012.11-x86.msm (which I want to read with msiinfo from msitools):

http://cdn.gstreamer.com/windows/x86/gstreamer-sdk-x86-2012.11-merge-modules.zip

Note that "gsf dump" will succeed, since it reads only the amount needed and not multiple of block size (the blocks are consecutive). Otoh, msitools reads by chunks and will hit that bug.
Comment 8 Morten Welinder 2012-12-22 15:50:56 UTC
> Otoh, msitools reads by chunks and will hit that bug.

Doesn't that basically mean that the file is broken and that GStreamer
has a bug it needs to fix?  (I hope GStreamer didn't use libgsf to write it!)
Comment 9 Marc-Andre Lureau 2012-12-22 16:24:20 UTC
(In reply to comment #8)
> > Otoh, msitools reads by chunks and will hit that bug.
> 
> Doesn't that basically mean that the file is broken and that GStreamer
> has a bug it needs to fix?  (I hope GStreamer didn't use libgsf to write it!)

I am pretty sure they used microsoft MSI tools, since there is nothing else out there. But tbh, I don't see how the patch would hurt anyway, and would still be compatible with "broken files"
Comment 10 Marc-Andre Lureau 2012-12-23 15:56:32 UTC
One more point in favour of fixing this, the Microsoft tools (Orca and msi* programs, installer..) have no issue dealing with those files.
Comment 11 Marc-Andre Lureau 2013-01-16 21:38:19 UTC
Morten, I believe the patch is not going to break any usage, it will only limit the read to what is actually needed. Even if the sector to read is not of sector-size, I don't think there is a valid argument to not allow that read.

I hope that can convince you. If not, we will sadly not be able to read some existing files out there, although other tools, including from Microsoft, do accept it fine.

A warning could be useful though. Would you accept an updated patch with that?
Comment 12 Morten Welinder 2013-01-21 16:45:55 UTC
I'll get around to it.
Comment 13 Marc-Andre Lureau 2013-02-13 21:02:58 UTC
Hi Morten, I believe those patches are worth not being ignored.

thanks again
Comment 14 Morten Welinder 2013-02-14 00:19:32 UTC
They aren't being ignored.  With only 14 open bugs, they also will not fall
through the cracks.  It'll happen sometime before the next release.
Comment 15 Marc-Andre Lureau 2013-02-14 00:57:42 UTC
cool, thanks
Comment 16 Morten Welinder 2013-02-23 02:05:36 UTC
This problem has been fixed in our software repository. The fix will go into the next software release. Thank you for your bug report.