[Git][reproducible-builds/strip-nondeterminism][master] Catch invalid ZIP "local" field lengths; we were previously inherently blindly…

[Chris Lamb]

Chris Lamb pushed to branch master at Reproducible Builds / strip-nondeterminism


Commits:
e5f5008e by Chris Lamb at 2018-11-09T15:59:50Z
Catch invalid ZIP "local" field lengths; we were previously inherently blindly the value supplied in the ZIP file. Thanks to Daniel Stender for the report. (Closes: #803503)

- - - - -


3 changed files:

- lib/File/StripNondeterminism/handlers/zip.pm
- + t/fixtures/zip/bug_803503.zip.in
- + t/fixtures/zip/bug_803503.zip.out


Changes:

=====================================
lib/File/StripNondeterminism/handlers/zip.pm
=====================================
@@ -76,11 +76,6 @@ sub normalize_member($$) {
 	return 1;
 }
 
-use constant {
-	CENTRAL_HEADER => 0,
-	LOCAL_HEADER => 1
-};
-
 sub unixtime_to_winnt($) {
 	my $unixtime = shift || 0;
 
@@ -91,11 +86,10 @@ sub unixtime_to_winnt($) {
 	return $unixtime + $secondsdiff;
 }
 
-sub normalize_extra_fields($$$) {
+sub normalize_extra_fields($$) {
 	# See http://sources.debian.net/src/zip/3.0-6/proginfo/extrafld.txt for extra field documentation
-	# $header_type is CENTRAL_HEADER or LOCAL_HEADER.
 	# WARNING: some fields have a different format depending on the header type
-	my ($canonical_time, $field, $header_type) = @_;
+	my ($canonical_time, $field) = @_;
 
 	my $result = "";
 	my $pos = 0;
@@ -149,6 +143,12 @@ sub normalize_extra_fields($$$) {
 				$result .= substr($field, $pos + 4, $len);
 			}
 		} else {
+			# Catch invalid field lengths by calculating whether we would
+			# read beyond the end of the file.
+			if ($pos + $len >= length($field)) {
+				warn "strip-nondeterminism: invalid extra field length ($len)";
+				return;
+			}
 			# use the current extra field unmodified.
 			$result .= substr($field, $pos, $len+4);
 		}
@@ -209,10 +209,11 @@ sub normalize {
 				? oct(755)
 				: oct(644));
 		}
-		$member->cdExtraField(
-			normalize_extra_fields($canonical_time, $member->cdExtraField(), CENTRAL_HEADER));
-		$member->localExtraField(
-			normalize_extra_fields($canonical_time, $member->localExtraField(), LOCAL_HEADER));
+		foreach my $x (qw(cdExtraField localExtraField)) {
+			my $result = normalize_extra_fields($canonical_time, $member->$x);
+			return 0 unless defined $result;
+			$member->$x($result);
+		}
 	}
 	my $old_perms = (stat($zip_filename))[2] & oct(7777);
 	$zip->overwrite();


=====================================
t/fixtures/zip/bug_803503.zip.in
=====================================
Binary files /dev/null and b/t/fixtures/zip/bug_803503.zip.in differ


=====================================
t/fixtures/zip/bug_803503.zip.out
=====================================
@@ -0,0 +1 @@
+bug_803503.zip.in
\ No newline at end of file



View it on GitLab: https://salsa.debian.org/reproducible-builds/strip-nondeterminism/commit/e5f5008eef0dbff52a5855c1c06bf729236a6684

-- 
View it on GitLab: https://salsa.debian.org/reproducible-builds/strip-nondeterminism/commit/e5f5008eef0dbff52a5855c1c06bf729236a6684
You're receiving this email because of your account on salsa.debian.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.reproducible-builds.org/pipermail/rb-commits/attachments/20181109/c6c30e99/attachment.html>