Skip to content

Commit 9d02a20

Browse files
olszomalmtrojnar
authored andcommitted
Fix unsafe ZIP size handling and allocation checks
1 parent f190ec5 commit 9d02a20

File tree

1 file changed

+37
-5
lines changed

1 file changed

+37
-5
lines changed

appx.c

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1856,6 +1856,11 @@ static size_t zipReadFileData(ZIP_FILE *zip, uint8_t **pData, ZIP_CENTRAL_DIRECT
18561856
}
18571857
if (entry->overrideData) {
18581858
compressedSize = entry->overrideData->compressedSize;
1859+
/* Validate sizes for safe allocation */
1860+
if (compressedSize > (uint64_t)(SIZE_MAX - 1)) {
1861+
fprintf(stderr, "Corrupted compressedSize : %" PRIu64"\n", compressedSize);
1862+
return 0; /* FAILED */
1863+
}
18591864
uncompressedSize = entry->overrideData->uncompressedSize;
18601865
compressedData = OPENSSL_zalloc(compressedSize + 1);
18611866
memcpy(compressedData, entry->overrideData->data, compressedSize);
@@ -1889,8 +1894,10 @@ static size_t zipReadFileData(ZIP_FILE *zip, uint8_t **pData, ZIP_CENTRAL_DIRECT
18891894
header.fileName = NULL;
18901895
header.extraField = NULL;
18911896

1892-
if (compressedSize > (uint64_t)zip->fileSize - entry->offsetOfLocalHeader) {
1893-
fprintf(stderr, "Corrupted compressedSize : 0x%08" PRIX64 "\n", entry->compressedSize);
1897+
/* Validate sizes for safe allocation */
1898+
if (compressedSize > (uint64_t)(SIZE_MAX - 1)
1899+
|| compressedSize > (uint64_t)zip->fileSize - entry->offsetOfLocalHeader) {
1900+
fprintf(stderr, "Corrupted compressedSize : %" PRIu64"\n", compressedSize);
18941901
return 0; /* FAILED */
18951902
}
18961903
compressedData = OPENSSL_zalloc(compressedSize + 1);
@@ -1909,11 +1916,24 @@ static size_t zipReadFileData(ZIP_FILE *zip, uint8_t **pData, ZIP_CENTRAL_DIRECT
19091916
*pData = compressedData;
19101917
dataSize = compressedSize;
19111918
} else if (entry->compression == COMPRESSION_DEFLATE) {
1912-
uint8_t *uncompressedData = OPENSSL_zalloc(uncompressedSize + 1);
1913-
uint64_t destLen = uncompressedSize;
1914-
uint64_t sourceLen = compressedSize;
1919+
uint8_t *uncompressedData;
1920+
uint64_t destLen, sourceLen;
19151921
int ret;
19161922

1923+
/* Validate sizes for safe allocation */
1924+
if (uncompressedSize > (uint64_t)(SIZE_MAX - 1)) {
1925+
fprintf(stderr, "Corrupted uncompressedSize : %" PRIu64"\n", uncompressedSize);
1926+
return 0; /* FAILED */
1927+
}
1928+
/* Detect suspicious compression ratio (zip bomb protection) */
1929+
if (uncompressedSize > 1024 * 1024 && uncompressedSize / 100 >= compressedSize) {
1930+
fprintf(stderr, "Error: suspicious compression ratio\n");
1931+
return 0; /* FAILED */
1932+
}
1933+
uncompressedData = OPENSSL_zalloc(uncompressedSize + 1);
1934+
destLen = uncompressedSize;
1935+
sourceLen = compressedSize;
1936+
19171937
ret = zipInflate(uncompressedData, &destLen, compressedData, (uLong *)&sourceLen);
19181938
OPENSSL_free(compressedData);
19191939

@@ -1980,6 +2000,8 @@ static int zipReadLocalHeader(ZIP_LOCAL_HEADER *header, ZIP_FILE *zip, uint64_t
19802000
header->extraFieldLen = fileGetU16(file);
19812001
/* file name (variable size) */
19822002
if (header->fileNameLen > 0) {
2003+
/* fileNameLen is uint16_t (ZIP spec, 2-byte field),
2004+
* so fileNameLen + 1 cannot overflow size_t */
19832005
header->fileName = OPENSSL_zalloc(header->fileNameLen + 1);
19842006
size = fread(header->fileName, 1, header->fileNameLen, file);
19852007
if (size != header->fileNameLen) {
@@ -1991,6 +2013,8 @@ static int zipReadLocalHeader(ZIP_LOCAL_HEADER *header, ZIP_FILE *zip, uint64_t
19912013
}
19922014
/* extra field (variable size) */
19932015
if (header->extraFieldLen > 0) {
2016+
/* extraFieldLen is uint16_t (ZIP spec, 2-byte field),
2017+
* so extraFieldLen + 1 cannot overflow size_t */
19942018
header->extraField = OPENSSL_zalloc(header->extraFieldLen + 1);
19952019
size = fread(header->extraField, 1, header->extraFieldLen, file);
19962020
if (size != header->extraFieldLen) {
@@ -2489,6 +2513,8 @@ static ZIP_CENTRAL_DIRECTORY_ENTRY *zipReadNextCentralDirectoryEntry(FILE *file)
24892513
entry->offsetOfLocalHeader = fileGetU32(file);
24902514
/* file name (variable size) */
24912515
if (entry->fileNameLen > 0) {
2516+
/* fileNameLen is uint16_t (ZIP spec, 2-byte field),
2517+
* so fileNameLen + 1 cannot overflow size_t */
24922518
entry->fileName = OPENSSL_zalloc(entry->fileNameLen + 1);
24932519
size = fread(entry->fileName, 1, entry->fileNameLen, file);
24942520
if (size != entry->fileNameLen) {
@@ -2499,6 +2525,8 @@ static ZIP_CENTRAL_DIRECTORY_ENTRY *zipReadNextCentralDirectoryEntry(FILE *file)
24992525
}
25002526
/* extra field (variable size) */
25012527
if (entry->extraFieldLen > 0) {
2528+
/* extraFieldLen is uint16_t (ZIP spec, 2-byte field),
2529+
* so extraFieldLen + 1 cannot overflow size_t */
25022530
entry->extraField = OPENSSL_zalloc(entry->extraFieldLen + 1);
25032531
size = fread(entry->extraField, 1, entry->extraFieldLen, file);
25042532
if (size != entry->extraFieldLen) {
@@ -2509,6 +2537,8 @@ static ZIP_CENTRAL_DIRECTORY_ENTRY *zipReadNextCentralDirectoryEntry(FILE *file)
25092537
}
25102538
/* file comment (variable size) */
25112539
if (entry->fileCommentLen > 0) {
2540+
/* fileCommentLen is uint16_t (ZIP spec, 2-byte field),
2541+
* so fileCommentLen + 1 cannot overflow size_t */
25122542
entry->fileComment = OPENSSL_zalloc(entry->fileCommentLen + 1);
25132543
size = fread(entry->fileComment, 1, entry->fileCommentLen, file);
25142544
if (size != entry->fileCommentLen) {
@@ -2647,6 +2677,8 @@ static int readZipEOCDR(ZIP_EOCDR *eocdr, FILE *file)
26472677
}
26482678
#endif
26492679
if (eocdr->commentLen > 0) {
2680+
/* ZIP_EOCDR commentLen is uint16_t (ZIP spec, 2-byte field),
2681+
* so fileCommentLen + 1 cannot overflow size_t */
26502682
eocdr->comment = OPENSSL_zalloc(eocdr->commentLen + 1);
26512683
size = fread(eocdr->comment, 1, eocdr->commentLen, file);
26522684
if (size != eocdr->commentLen) {

0 commit comments

Comments
 (0)