php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #70096 Repeated iptcembed() adds superfluous FF bytes
Submitted: 2015-07-18 19:06 UTC Modified: 2015-07-18 19:42 UTC
From: cmb@php.net Assigned: cmb
Status: Closed Package: GetImageSize related
PHP Version: 7.0.0beta1 OS: *
Private report: No CVE-ID:
 [2015-07-18 19:06 UTC] cmb@php.net
Description:
------------
Repeately calling iptcembed() causes a superfluous FF byte to be
added with each call. While this may not be a violation of the
JFIF standard (the specs are not publicly available, apparently),
I'd nonetheless consider this a bug, because repeated embedding of
the same metadata should not result in different files.


Test script:
---------------
<?php
$filename = __DIR__ . '/test.jpg';
@unlink($filename);
$im = imagecreatetruecolor(10, 10);
imagejpeg($im, $filename);
imagedestroy($im);
$data = "\x1C\x02x\x00\x0ATest image"
    . "\x1C\x02t\x00\x22Copyright 2008-2009, The PHP Group";
$content = iptcembed($data, $filename);
$fp = fopen($filename, "wb");
fwrite($fp, $content);
fclose($fp);
echo filesize($filename), PHP_EOL;
clearstatcache();
$content = iptcembed($data, $filename);
$fp = fopen($filename, "wb");
fwrite($fp, $content);
fclose($fp);
echo filesize($filename), PHP_EOL;
?>


Expected result:
----------------
779
779


Actual result:
--------------
779
780


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-07-18 19:06 UTC] cmb@php.net
-Assigned To: +Assigned To: cmb
 [2015-07-18 19:42 UTC] cmb@php.net
-Status: Assigned +Status: Analyzed
 [2015-07-18 19:42 UTC] cmb@php.net
The algorithm to embed an APP13 marker segment into the file works
by processing the marker segments, and to copy them verbatim. The
only special handling is that after the first APP0 or APP1 marker
segment the new APP13 marker segment is inserted, and an existing
APP13 marker segment is skipped. After the latter has been done,
the rest of the stream is copied in one go; no need for further
processing the individual marker segments.

However, when an existing APP13 is recognized[1], its first byte
(FF) has already been copied to the buffer[2], and only the rest
of the marker segment is skipped. This results in duplication of
this byte.

[1] <https://github.com/php/php-src/blob/php-5.6.11/ext/standard/iptc.c#L236>
[2] <https://github.com/php/php-src/blob/php-5.6.11/ext/standard/iptc.c#L227>
 [2015-07-18 20:58 UTC] cmb@php.net
Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8c483ce36cb07d7e986484da93cf064ad1cc9dd7
Log: Fix #70096: Repeated iptcembed() adds superfluous FF bytes
 [2015-07-18 20:58 UTC] cmb@php.net
-Status: Analyzed +Status: Closed
 [2015-07-21 14:20 UTC] ab@php.net
Automatic comment on behalf of cmb
Revision: http://git.php.net/?p=php-src.git;a=commit;h=8c483ce36cb07d7e986484da93cf064ad1cc9dd7
Log: Fix #70096: Repeated iptcembed() adds superfluous FF bytes
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC