Skip to content

Commit 758daf7

Browse files
fkjellbergfkjellberg
committed
Better handling of corrupt archives
1 parent 6f6698c commit 758daf7

2 files changed

Lines changed: 66 additions & 17 deletions

File tree

‎src/main/java/org/apache/commons/compress/archivers/lha/LhaArchiveInputStream.java‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -343,12 +343,13 @@ LhaArchiveEntry readHeader() throws IOException {
343343
* @throws IOException
344344
*/
345345
LhaArchiveEntry readHeaderLevel0(ByteBuffer buffer) throws IOException {
346-
final int headerSize = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_0_OFFSET_HEADER_SIZE));
346+
// Add two to the header size as the first two bytes are not included
347+
final int headerSize = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_0_OFFSET_HEADER_SIZE)) + 2;
347348
if (headerSize < HEADER_GENERIC_MINIMUM_HEADER_LENGTH) {
348349
throw new ArchiveException("Invalid header level 0 length: %d", headerSize);
349350
}
350351

351-
buffer = readRemainingHeaderData(buffer, headerSize + 2); // Header size is not including the first two bytes of the header
352+
buffer = readRemainingHeaderData(buffer, headerSize);
352353

353354
final int headerChecksum = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_0_OFFSET_HEADER_CHECKSUM));
354355

@@ -361,6 +362,12 @@ LhaArchiveEntry readHeaderLevel0(ByteBuffer buffer) throws IOException {
361362
.setLastModifiedDate(new Date(ZipUtil.dosToJavaTime(Integer.toUnsignedLong(buffer.getInt(HEADER_LEVEL_0_OFFSET_LAST_MODIFIED_DATE_TIME)))));
362363

363364
final int filenameLength = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_0_OFFSET_FILENAME_LENGTH));
365+
366+
// Make sure the filename is not overflowing into the CRC field
367+
if (filenameLength > (headerSize - HEADER_LEVEL_0_OFFSET_FILENAME - 2)) {
368+
throw new ArchiveException("Invalid pathname length");
369+
}
370+
364371
buffer.position(HEADER_LEVEL_0_OFFSET_FILENAME);
365372
entryBuilder.setFilename(getPathname(buffer, filenameLength))
366373
.setDirectory(isDirectory(compressionMethod))
@@ -385,12 +392,13 @@ LhaArchiveEntry readHeaderLevel0(ByteBuffer buffer) throws IOException {
385392
* @throws IOException
386393
*/
387394
LhaArchiveEntry readHeaderLevel1(ByteBuffer buffer) throws IOException {
388-
final int baseHeaderSize = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_1_OFFSET_BASE_HEADER_SIZE));
395+
// Add two to the header size as the first two bytes are not included
396+
final int baseHeaderSize = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_1_OFFSET_BASE_HEADER_SIZE)) + 2;
389397
if (baseHeaderSize < HEADER_GENERIC_MINIMUM_HEADER_LENGTH) {
390398
throw new ArchiveException("Invalid header level 1 length: %d", baseHeaderSize);
391399
}
392400

393-
buffer = readRemainingHeaderData(buffer, baseHeaderSize + 2);
401+
buffer = readRemainingHeaderData(buffer, baseHeaderSize);
394402

395403
final int baseHeaderChecksum = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_1_OFFSET_BASE_HEADER_CHECKSUM));
396404

@@ -403,6 +411,14 @@ LhaArchiveEntry readHeaderLevel1(ByteBuffer buffer) throws IOException {
403411
.setLastModifiedDate(new Date(ZipUtil.dosToJavaTime(Integer.toUnsignedLong(buffer.getInt(HEADER_LEVEL_1_OFFSET_LAST_MODIFIED_DATE_TIME)))));
404412

405413
final int filenameLength = Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_1_OFFSET_FILENAME_LENGTH));
414+
415+
// Make sure the filename is not overflowing into the CRC, OS ID and first extended header length fields.
416+
// This check is not bulletproof because there might also be an extended area after the filename that
417+
// we cannot detect for corrupt archives.
418+
if (filenameLength > (baseHeaderSize - HEADER_LEVEL_1_OFFSET_FILENAME - 5)) {
419+
throw new ArchiveException("Invalid pathname length");
420+
}
421+
406422
buffer.position(HEADER_LEVEL_1_OFFSET_FILENAME);
407423
entryBuilder.setFilename(getPathname(buffer, filenameLength))
408424
.setDirectory(isDirectory(compressionMethod))
@@ -418,16 +434,17 @@ LhaArchiveEntry readHeaderLevel1(ByteBuffer buffer) throws IOException {
418434
final List<ByteBuffer> headerParts = new ArrayList<>();
419435
headerParts.add(buffer);
420436

421-
int nextHeaderSize = Short.toUnsignedInt(buffer.getShort());
422-
while (nextHeaderSize > 0) {
423-
final ByteBuffer extendedHeaderBuffer = readExtendedHeader(nextHeaderSize);
424-
skipSize -= nextHeaderSize;
437+
buffer.position(baseHeaderSize - 2); // First extended header length is at the end of the base header
438+
int extendedHeaderSize = Short.toUnsignedInt(buffer.getShort());
439+
while (extendedHeaderSize > 0) {
440+
final ByteBuffer extendedHeaderBuffer = readExtendedHeader(extendedHeaderSize);
441+
skipSize -= extendedHeaderSize;
425442

426443
parseExtendedHeader(extendedHeaderBuffer, entryBuilder);
427444

428445
headerParts.add(extendedHeaderBuffer);
429446

430-
nextHeaderSize = Short.toUnsignedInt(extendedHeaderBuffer.getShort(extendedHeaderBuffer.limit() - 2));
447+
extendedHeaderSize = Short.toUnsignedInt(extendedHeaderBuffer.getShort(extendedHeaderBuffer.limit() - 2));
431448
}
432449

433450
entryBuilder.setCompressedSize(skipSize);
@@ -473,17 +490,21 @@ LhaArchiveEntry readHeaderLevel2(ByteBuffer buffer) throws IOException {
473490
.setCrcValue(Short.toUnsignedInt(buffer.getShort(HEADER_LEVEL_2_OFFSET_CRC)))
474491
.setOsId(Byte.toUnsignedInt(buffer.get(HEADER_LEVEL_2_OFFSET_OS_ID)));
475492

476-
int extendedHeaderOffset = HEADER_LEVEL_2_OFFSET_FIRST_EXTENDED_HEADER_SIZE;
477-
int nextHeaderSize = Short.toUnsignedInt(buffer.getShort(extendedHeaderOffset));
478-
while (nextHeaderSize > 0) {
493+
int extendedHeaderSize = Short.toUnsignedInt(buffer.getShort(HEADER_LEVEL_2_OFFSET_FIRST_EXTENDED_HEADER_SIZE));
494+
int extendedHeaderOffset = HEADER_LEVEL_2_OFFSET_FIRST_EXTENDED_HEADER_SIZE + 2;
495+
while (extendedHeaderSize > 0) {
496+
if ((extendedHeaderOffset + extendedHeaderSize) > buffer.limit()) {
497+
throw new ArchiveException("Invalid extended header length");
498+
}
499+
479500
// Create new ByteBuffer as a slice from the full header. Set limit to the extended header length.
480-
final ByteBuffer extendedHeaderBuffer = byteBufferSlice(buffer, extendedHeaderOffset + 2, nextHeaderSize).order(ByteOrder.LITTLE_ENDIAN);
501+
final ByteBuffer extendedHeaderBuffer = byteBufferSlice(buffer, extendedHeaderOffset, extendedHeaderSize).order(ByteOrder.LITTLE_ENDIAN);
481502

482-
extendedHeaderOffset += nextHeaderSize;
503+
extendedHeaderOffset += extendedHeaderSize;
483504

484505
parseExtendedHeader(extendedHeaderBuffer, entryBuilder);
485506

486-
nextHeaderSize = Short.toUnsignedInt(extendedHeaderBuffer.getShort(extendedHeaderBuffer.limit() - 2));
507+
extendedHeaderSize = Short.toUnsignedInt(extendedHeaderBuffer.getShort(extendedHeaderBuffer.limit() - 2));
487508
}
488509

489510
final LhaArchiveEntry entry = entryBuilder.get();

‎src/test/java/org/apache/commons/compress/archivers/lha/LhaArchiveInputStreamTest.java‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ void testInvalidHeaderLevel0Length() throws IOException {
254254
archive.getNextEntry();
255255
fail("Expected ArchiveException for invalid header length");
256256
} catch (ArchiveException e) {
257-
assertEquals("Invalid header level 0 length: 16", e.getMessage());
257+
assertEquals("Invalid header level 0 length: 18", e.getMessage());
258258
}
259259
}
260260

@@ -272,6 +272,20 @@ void testInvalidHeaderLevel0Checksum() throws IOException {
272272
}
273273
}
274274

275+
@Test
276+
void testInvalidHeaderLevel0FilenameLength() throws IOException {
277+
final byte[] data = toByteArray(VALID_HEADER_LEVEL_0_FILE);
278+
279+
data[21] = 22; // Change the length of the filename
280+
281+
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(new ByteArrayInputStream(data)).get()) {
282+
archive.getNextEntry();
283+
fail("Expected ArchiveException for invalid filename");
284+
} catch (ArchiveException e) {
285+
assertEquals("Invalid pathname length", e.getMessage());
286+
}
287+
}
288+
275289
@Test
276290
void testParseHeaderLevel0FileWithFoldersMacos() throws IOException {
277291
// The lha file was generated by LHa for UNIX version 1.14i-ac20211125 for Macos
@@ -571,7 +585,7 @@ void testInvalidHeaderLevel1Length() throws IOException {
571585
archive.getNextEntry();
572586
fail("Expected ArchiveException for invalid header length");
573587
} catch (ArchiveException e) {
574-
assertEquals("Invalid header level 1 length: 16", e.getMessage());
588+
assertEquals("Invalid header level 1 length: 18", e.getMessage());
575589
}
576590
}
577591

@@ -605,6 +619,20 @@ void testInvalidHeaderLevel1Crc() throws IOException {
605619
}
606620
}
607621

622+
@Test
623+
void testInvalidHeaderLevel1FilenameLength() throws IOException {
624+
final byte[] data = toByteArray(VALID_HEADER_LEVEL_1_FILE);
625+
626+
data[21] = 10; // Change the length of the filename
627+
628+
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(new ByteArrayInputStream(data)).get()) {
629+
archive.getNextEntry();
630+
fail("Expected ArchiveException for invalid filename");
631+
} catch (ArchiveException e) {
632+
assertEquals("Invalid pathname length", e.getMessage());
633+
}
634+
}
635+
608636
@Test
609637
void testParseHeaderLevel1FileWithFoldersMacos() throws IOException {
610638
// The lha file was generated by LHa for UNIX version 1.14i-ac20211125 for Macos

0 commit comments

Comments
 (0)