libzipfile: add additional validity checks.
- Make sure the start and end of the data for a given entry are
within the allocated buffer.
- Make sure all central directory entries start and end within the
central directory buffer.
- Reject zip file entries that have no filenames.
bug: 30916186
test: test_zipfile with known bad zip files.
Change-Id: Ibf3f6469e60c85ec1608f5ce613d40867d2d09b7
diff --git a/libzipfile/centraldir.c b/libzipfile/centraldir.c
index 911e2b9..2ce73e8 100644
--- a/libzipfile/centraldir.c
+++ b/libzipfile/centraldir.c
@@ -60,16 +60,20 @@
return 0;
}
+static const int kCompressionStored = 0x0;
+static const int kCompressionDeflate = 0x8;
+
static int
read_central_directory_entry(Zipfile* file, Zipentry* entry,
const unsigned char** buf, ssize_t* len)
{
const unsigned char* p;
+ size_t remaining;
+ const unsigned char* bufLimit;
unsigned short versionMadeBy;
unsigned short versionToExtract;
unsigned short gpBitFlag;
- unsigned short compressionMethod;
unsigned short lastModFileTime;
unsigned short lastModFileDate;
unsigned long crc32;
@@ -84,8 +88,9 @@
unsigned int dataOffset;
unsigned short lfhExtraFieldSize;
-
p = *buf;
+ remaining = *len;
+ bufLimit = file->buf + file->bufsize;
if (*len < ENTRY_LEN) {
fprintf(stderr, "cde entry not large enough\n");
@@ -115,41 +120,88 @@
localHeaderRelOffset = read_le_int(&p[0x2a]);
p += ENTRY_LEN;
+ remaining -= ENTRY_LEN;
// filename
if (entry->fileNameLength != 0) {
+ if (entry->fileNameLength > remaining) {
+ fprintf(stderr, "cde entry not large enough for file name.\n");
+ return 1;
+ }
+
entry->fileName = p;
} else {
- entry->fileName = NULL;
+ fprintf(stderr, "cde entry does not contain a file name.\n");
+ return 1;
}
p += entry->fileNameLength;
+ remaining -= entry->fileNameLength;
- // extra field
+ // extra field, if any
if (extraFieldLength != 0) {
+ if (extraFieldLength > remaining) {
+ fprintf(stderr, "cde entry not large enough for extra field.\n");
+ return 1;
+ }
+
extraField = p;
} else {
extraField = NULL;
}
p += extraFieldLength;
+ remaining -= extraFieldLength;
// comment, if any
if (fileCommentLength != 0) {
+ if (fileCommentLength > remaining) {
+ fprintf(stderr, "cde entry not large enough for file comment.\n");
+ return 1;
+ }
+
fileComment = p;
} else {
fileComment = NULL;
}
p += fileCommentLength;
+ remaining -= fileCommentLength;
*buf = p;
+ *len = remaining;
// the size of the extraField in the central dir is how much data there is,
// but the one in the local file header also contains some padding.
p = file->buf + localHeaderRelOffset;
+ if (p >= bufLimit) {
+ fprintf(stderr, "Invalid local header offset for entry.\n");
+ return 1;
+ }
+
extraFieldLength = read_le_short(&p[0x1c]);
dataOffset = localHeaderRelOffset + LFH_SIZE
+ entry->fileNameLength + extraFieldLength;
entry->data = file->buf + dataOffset;
+
+ // Sanity check: make sure that the start of the entry data is within
+ // our allocated buffer.
+ if ((entry->data < file->buf) || (entry->data >= bufLimit)) {
+ fprintf(stderr, "Invalid data offset for entry.\n");
+ return 1;
+ }
+
+ // Sanity check: make sure that the end of the entry data is within
+ // our allocated buffer. We need to look at the uncompressedSize for
+ // stored entries and the compressed size for deflated entries.
+ if ((entry->compressionMethod == kCompressionStored) &&
+ (entry->uncompressedSize > (unsigned int) (bufLimit - entry->data))) {
+ fprintf(stderr, "Invalid uncompressed size for stored entry.\n");
+ return 1;
+ }
+ if ((entry->compressionMethod == kCompressionDeflate) &&
+ (entry->compressedSize > (unsigned int) (bufLimit - entry->data))) {
+ fprintf(stderr, "Invalid uncompressed size for deflated entry.\n");
+ return 1;
+ }
#if 0
printf("file->buf=%p entry->data=%p dataOffset=%x localHeaderRelOffset=%d "
"entry->fileNameLength=%d extraFieldLength=%d\n",
diff --git a/libzipfile/test_zipfile.c b/libzipfile/test_zipfile.c
index 1aaa913..a2f6bc7 100644
--- a/libzipfile/test_zipfile.c
+++ b/libzipfile/test_zipfile.c
@@ -75,7 +75,6 @@
unsize = get_zipentry_size(entry);
size = unsize * 1.001;
scratch = malloc(size);
- printf("scratch=%p\n", scratch);
err = decompress_zipentry(entry, scratch, size);
if (err != 0) {
fprintf(stderr, "error decompressing file\n");
