[Reproducible-commits] [strip-nondeterminism] 02/03: Rewrite PNG handler to support bailing out on invalid header lengths

[Chris Lamb]

This is an automated email from the git hooks/post-receive script.

lamby pushed a commit to branch master
in repository strip-nondeterminism.

commit 311c913d452e1811531c2bfeeb631c024819d19a
Author: Chris Lamb <lamby at debian.org>
Date:   Tue Jul 19 18:29:51 2016 +0200

    Rewrite PNG handler to support bailing out on invalid header lengths
---
 lib/File/StripNondeterminism/handlers/png.pm | 60 +++++++++++++++++++++-------
 1 file changed, 45 insertions(+), 15 deletions(-)

diff --git a/lib/File/StripNondeterminism/handlers/png.pm b/lib/File/StripNondeterminism/handlers/png.pm
index 6dcf204..7fd0d7c 100644
--- a/lib/File/StripNondeterminism/handlers/png.pm
+++ b/lib/File/StripNondeterminism/handlers/png.pm
@@ -50,16 +50,33 @@ sub text_chunk {
 sub normalize {
 	my ($filename) = @_;
 
-	my $canonical_time = $File::StripNondeterminism::canonical_time;
-
 	my $tempfile = File::Temp->new(DIR => dirname($filename));
 
+	open(my $fh, '+<', $filename) or die "$filename: open: $!";
+
+	if (_normalize($filename, $fh, $tempfile)) {
+		chmod((stat($fh))[2] & 07777, $tempfile->filename);
+		rename($tempfile->filename, $filename)
+			or die "$filename: unable to overwrite: rename: $!";
+	}
+
+	$tempfile->unlink_on_destroy(0);
+
+	close $fh;
+}
+
+sub _normalize {
+	my ($filename, $fh, $tempfile) = @_;
+
+	my $canonical_time = $File::StripNondeterminism::canonical_time;
+
 	my $buf;
+	my $changed;
 	my $bytes_read;
 
-	open(my $fh, '+<', $filename) or die "$filename: open: $!";
 	read($fh, my $magic, 8); $magic eq "\x89PNG\r\n\x1a\n"
 		or die "$filename: does not appear to be a PNG";
+
 	print $tempfile $magic;
 
 	while (read($fh, my $header, 8) == 8) {
@@ -67,27 +84,45 @@ sub normalize {
 
 		# Include the trailing CRC when reading
 		$len += 4;
-		# We cannot trust the value of $len, so we only read(2) if it
-		# has a sane size.
+
+		# We cannot trust the value of $len so we cannot simply read
+		# that many bytes in memory. Therefore rely on a sane value
+		# for a "header" and hope that matches everything.
 		if ($len < 4096) {
-			read $fh, my $data, $len + 4;
+			my $bytes_read = read($fh, my $data, $len);
+
+			if ($bytes_read != $len) {
+				warn "$filename: invalid length in $type header";
+				return 0;
+			}
 
 			if ($type eq "tIME") {
 				print $tempfile time_chunk($canonical_time) if defined($canonical_time);
+				$changed = 1;
 				next;
 			} elsif (($type =~ /[tiz]EXt/) && ($data =~ /^(date:[^\0]+|Creation Time)\0/)) {
 				print $tempfile text_chunk($1, strftime("%Y-%m-%dT%H:%M:%S-00:00",
 								gmtime($canonical_time))) if defined($canonical_time);
+				$changed = 1;
 				next;
 			}
 		}
 
-		# Read/write in chunks
 		print $tempfile $header;
-		while (($len > 0) && ($bytes_read = read($fh, $buf, 4096))) {
-			$len = $len - $bytes_read;
+
+		while ($len > 0) {
+			# Can't trust $len so read data part in chunks
+			$bytes_read = read($fh, $buf, 4096);
+
+			if ($bytes_read == 0) {
+				warn "$filename: invalid length in $type header";
+				return 0;
+			}
+
 			print $tempfile $buf;
+			$len -= $bytes_read;
 		}
+		defined($bytes_read) or die "$filename: read failed: $!";
 
 		# Stop processing immediately in case there's garbage after the
 		# PNG datastream. (https://bugs.debian.org/802057)
@@ -103,12 +138,7 @@ sub normalize {
 	}
 	defined($bytes_read) or die "$filename: read failed: $!";
 
-	chmod((stat($fh))[2] & 07777, $tempfile->filename);
-	rename($tempfile->filename, $filename)
-		or die "$filename: unable to overwrite: rename: $!";
-	$tempfile->unlink_on_destroy(0);
-
-	close $fh;
+	return $changed;
 }
 
 1;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/reproducible/strip-nondeterminism.git