From dd99585d6f83374249fed2337cd1a01bd126f70d Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp@cs.stanford.edu>
Date: Mon, 20 Feb 2023 12:01:16 -0800
Subject: [PATCH] zip-reader: Report corrupted Zip archives.

---
 src/libpspp/zip-reader.c | 39 +++++++++++++++++++++++++++------------
 tests/libpspp/zip.at     | 23 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/src/libpspp/zip-reader.c b/src/libpspp/zip-reader.c
index 00249e0bae..38cba29566 100644
--- a/src/libpspp/zip-reader.c
+++ b/src/libpspp/zip-reader.c
@@ -32,6 +32,7 @@
 #include "libpspp/integer-format.h"
 #include "libpspp/str.h"
 
+#include "gl/crc.h"
 #include "gl/xalloc.h"
 
 #include "gettext.h"
@@ -46,6 +47,10 @@ struct zip_member
   uint32_t offset;            /* Starting offset in file. */
   uint32_t comp_size;         /* Length of member file data, in bytes. */
   uint32_t ucomp_size;        /* Uncompressed length of member file data, in bytes. */
+
+  uint32_t expected_crc;
+  uint32_t accumulated_crc;
+
   const struct decompressor *decompressor;
 
   size_t bytes_unread;       /* Number of bytes left in the member available for reading */
@@ -93,6 +98,7 @@ struct zip_entry
   uint32_t offset;            /* Starting offset in file. */
   uint32_t comp_size;         /* Length of member file data, in bytes. */
   uint32_t ucomp_size;        /* Uncompressed length of member file data, in bytes. */
+  uint32_t expected_crc;      /* CRC32 of uncompressed data. */
   char *name;                 /* Name of member file. */
 };
 
@@ -230,10 +236,19 @@ zip_member_read (struct zip_member *zm, void *buf, size_t bytes)
     return 0;
 
   int bytes_read = zm->decompressor->read (zm, buf, bytes);
-  if (bytes_read < 0)
+  if (bytes_read <= 0)
     return bytes_read;
 
   zm->bytes_unread -= bytes_read;
+  zm->accumulated_crc = crc32_update (zm->accumulated_crc, buf, bytes_read);
+  if (!zm->bytes_unread && zm->accumulated_crc != zm->expected_crc)
+    {
+      zm->error = xasprintf (_("%s: corrupt archive reading member \"%s\": "
+                               "bad CRC %#"PRIx32" (expected %"PRIx32")"),
+                             zm->file_name, zm->member_name,
+                             zm->accumulated_crc, zm->expected_crc);
+      return -1;
+    }
 
   return bytes_read;
 }
@@ -291,7 +306,7 @@ zip_header_read_next (FILE *file, const char *file_name,
   get_u16 (file);       /* comp_type */
   get_u16 (file);       /* time */
   get_u16 (file);       /* date */
-  get_u32 (file);       /* expected_crc */
+  ze->expected_crc = get_u32 (file);
   ze->comp_size = get_u32 (file);
   ze->ucomp_size = get_u32 (file);
   uint16_t nlen = get_u16 (file);
@@ -444,16 +459,16 @@ zip_member_open (struct zip_reader *zr, const char *member,
                        zr->file_name, strerror (errno));
 
   struct zip_member *zm = xmalloc (sizeof *zm);
-  zm->file_name = xstrdup (zr->file_name);
-  zm->member_name = xstrdup (member);
-  zm->fp = fp;
-  zm->offset = ze->offset;
-  zm->comp_size = ze->comp_size;
-  zm->ucomp_size = ze->ucomp_size;
-  zm->decompressor = NULL;
-  zm->bytes_unread = ze->ucomp_size;
-  zm->aux = NULL;
-  zm->error = NULL;
+  *zm = (struct zip_member) {
+    .file_name = xstrdup (zr->file_name),
+    .member_name = xstrdup (member),
+    .fp = fp,
+    .offset = ze->offset,
+    .comp_size = ze->comp_size,
+    .ucomp_size = ze->ucomp_size,
+    .bytes_unread = ze->ucomp_size,
+    .expected_crc = ze->expected_crc,
+  };
 
   char *error;
   if (0 != fseeko (zm->fp, zm->offset, SEEK_SET))
diff --git a/tests/libpspp/zip.at b/tests/libpspp/zip.at
index 5b21d50f58..bcc550457a 100644
--- a/tests/libpspp/zip.at
+++ b/tests/libpspp/zip.at
@@ -57,6 +57,29 @@ AT_CHECK([
 ])
 AT_CLEANUP
 
+AT_SETUP([zip - detect corruption on unzip])
+AT_KEYWORDS([compression])
+mkdir write
+cd write
+AT_DATA([data.txt], [xyzzy
+])
+AT_CHECK([zip-test w ../foo.zip data.txt])
+cd ..
+
+mkdir extract
+cd extract
+AT_CHECK([zip-test r ../foo.zip data.txt])
+AT_CHECK([cat data.txt], [0], [xyzzy
+])
+cd ..
+
+mkdir error
+cd error
+sed 's/xyzzy/XYZZY/' < ../foo.zip > ../corrupted.zip
+AT_CHECK([zip-test r ../corrupted.zip data.txt], [1], [], [dnl
+Unzip failed: ../corrupted.zip: corrupt archive reading member "data.txt": bad CRC 0x2a2bcd20 (expected e1bd2a6e)
+])
+AT_CLEANUP
 
 AT_SETUP([zip to pipe])
 AT_KEYWORDS([compression])
-- 
2.30.2