Skip to content

Commit fad7c2c

Browse files
fkjellbergfkjellberg
committed
Add better length checks for pathnames and extended headers
1 parent 15cc54f commit fad7c2c

2 files changed

Lines changed: 159 additions & 4 deletions

File tree

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

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,22 @@ Builder setFileSeparatorChar(final char fileSeparatorChar) {
185185
private static final int EXTENDED_HEADER_TYPE_UNIX_UID_GID = 0x51;
186186
private static final int EXTENDED_HEADER_TYPE_UNIX_TIMESTAMP = 0x54;
187187

188+
/**
189+
* Length in bytes of the next extended header size field.
190+
*/
191+
private static final int EXTENDED_HEADER_NEXT_HEADER_SIZE_LENGTH = 2;
192+
193+
/**
194+
* Minimum extended header length.
195+
*/
196+
private static final int MIN_EXTENDED_HEADER_LENGTH = 3;
197+
198+
private static final int EXTENDED_HEADER_TYPE_COMMON_MIN_PAYLOAD_LENGTH = 2;
199+
private static final int EXTENDED_HEADER_TYPE_MSDOS_FILE_ATTRIBUTES_PAYLOAD_LENGTH = 2;
200+
private static final int EXTENDED_HEADER_TYPE_UNIX_PERMISSION_PAYLOAD_LENGTH = 2;
201+
private static final int EXTENDED_HEADER_TYPE_UNIX_UID_GID_PAYLOAD_LENGTH = 4;
202+
private static final int EXTENDED_HEADER_TYPE_UNIX_TIMESTAMP_PAYLOAD_LENGTH = 4;
203+
188204
// Compression methods
189205
private static final String COMPRESSION_METHOD_DIRECTORY = "-lhd-"; // Directory entry
190206
private static final String COMPRESSION_METHOD_LH0 = "-lh0-";
@@ -528,6 +544,10 @@ String getPathname(final ByteBuffer buffer, final int pathnameLength) throws Arc
528544
// Check pathname length to ensure we don't allocate too much memory
529545
if (pathnameLength > MAX_PATHNAME_LENGTH) {
530546
throw new ArchiveException("Pathname is longer than the maximum allowed (%d > %d)", pathnameLength, MAX_PATHNAME_LENGTH);
547+
} else if (pathnameLength < 0) {
548+
throw new ArchiveException("Pathname length is negative");
549+
} else if (pathnameLength > (buffer.limit() - buffer.position())) {
550+
throw new ArchiveException("Invalid pathname length");
531551
}
532552

533553
final byte[] pathnameBuffer = new byte[pathnameLength];
@@ -610,9 +630,18 @@ private ByteBuffer readExtendedHeader(final int headerSize) throws IOException {
610630
* @throws IOException
611631
*/
612632
void parseExtendedHeader(final ByteBuffer extendedHeaderBuffer, final LhaArchiveEntry.Builder entryBuilder) throws IOException {
633+
final int extendedHeaderLength = extendedHeaderBuffer.limit() - extendedHeaderBuffer.position();
634+
if (extendedHeaderLength < MIN_EXTENDED_HEADER_LENGTH) {
635+
throw new ArchiveException("Invalid extended header length");
636+
}
637+
613638
final int extendedHeaderType = Byte.toUnsignedInt(extendedHeaderBuffer.get());
614639
if (extendedHeaderType == EXTENDED_HEADER_TYPE_COMMON) {
615640
// Common header
641+
if (extendedHeaderLength < (MIN_EXTENDED_HEADER_LENGTH + EXTENDED_HEADER_TYPE_COMMON_MIN_PAYLOAD_LENGTH)) {
642+
throw new ArchiveException("Invalid extended header length");
643+
}
644+
616645
final int crcPos = extendedHeaderBuffer.position(); // Save the current position to be able to set the header CRC later
617646

618647
// Header CRC
@@ -622,14 +651,14 @@ void parseExtendedHeader(final ByteBuffer extendedHeaderBuffer, final LhaArchive
622651
extendedHeaderBuffer.putShort(crcPos, (short) 0);
623652
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_FILENAME) {
624653
// File name header
625-
final int filenameLength = extendedHeaderBuffer.limit() - extendedHeaderBuffer.position() - 2;
654+
final int filenameLength = extendedHeaderBuffer.limit() - extendedHeaderBuffer.position() - EXTENDED_HEADER_NEXT_HEADER_SIZE_LENGTH;
626655
final String filename = getPathname(extendedHeaderBuffer, filenameLength);
627656
entryBuilder.setFilename(filename);
628657
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_DIRECTORY_NAME) {
629658
// Directory name header
630-
final int directoryNameLength = extendedHeaderBuffer.limit() - extendedHeaderBuffer.position() - 2;
659+
final int directoryNameLength = extendedHeaderBuffer.limit() - extendedHeaderBuffer.position() - EXTENDED_HEADER_NEXT_HEADER_SIZE_LENGTH;
631660
final String directoryName = getPathname(extendedHeaderBuffer, directoryNameLength);
632-
if (directoryName.charAt(directoryName.length() - 1) != fileSeparatorChar) {
661+
if (directoryName.length() > 0 && directoryName.charAt(directoryName.length() - 1) != fileSeparatorChar) {
633662
// If the directory name does not end with a file separator, append it
634663
entryBuilder.setDirectoryName(directoryName + fileSeparatorChar);
635664
} else {
@@ -638,16 +667,32 @@ void parseExtendedHeader(final ByteBuffer extendedHeaderBuffer, final LhaArchive
638667

639668
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_MSDOS_FILE_ATTRIBUTES) {
640669
// MS-DOS file attributes
670+
if (extendedHeaderLength != (MIN_EXTENDED_HEADER_LENGTH + EXTENDED_HEADER_TYPE_MSDOS_FILE_ATTRIBUTES_PAYLOAD_LENGTH)) {
671+
throw new ArchiveException("Invalid extended header length");
672+
}
673+
641674
entryBuilder.setMsdosFileAttributes(Short.toUnsignedInt(extendedHeaderBuffer.getShort()));
642675
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_UNIX_PERMISSION) {
643676
// UNIX file permission
677+
if (extendedHeaderLength != (MIN_EXTENDED_HEADER_LENGTH + EXTENDED_HEADER_TYPE_UNIX_PERMISSION_PAYLOAD_LENGTH)) {
678+
throw new ArchiveException("Invalid extended header length");
679+
}
680+
644681
entryBuilder.setUnixPermissionMode(Short.toUnsignedInt(extendedHeaderBuffer.getShort()));
645682
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_UNIX_UID_GID) {
646683
// UNIX group/user ID
684+
if (extendedHeaderLength != (MIN_EXTENDED_HEADER_LENGTH + EXTENDED_HEADER_TYPE_UNIX_UID_GID_PAYLOAD_LENGTH)) {
685+
throw new ArchiveException("Invalid extended header length");
686+
}
687+
647688
entryBuilder.setUnixGroupId(Short.toUnsignedInt(extendedHeaderBuffer.getShort()));
648689
entryBuilder.setUnixUserId(Short.toUnsignedInt(extendedHeaderBuffer.getShort()));
649690
} else if (extendedHeaderType == EXTENDED_HEADER_TYPE_UNIX_TIMESTAMP) {
650691
// UNIX last modified time
692+
if (extendedHeaderLength != (MIN_EXTENDED_HEADER_LENGTH + EXTENDED_HEADER_TYPE_UNIX_TIMESTAMP_PAYLOAD_LENGTH)) {
693+
throw new ArchiveException("Invalid extended header length");
694+
}
695+
651696
entryBuilder.setLastModifiedDate(new Date(Integer.toUnsignedLong(extendedHeaderBuffer.getInt()) * 1000));
652697
}
653698

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

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1165,21 +1165,52 @@ void testParseHeaderLevel2FileWithMsdosAttributes() throws IOException {
11651165
}
11661166
}
11671167

1168+
@Test
1169+
void testParseExtendedHeaderTooShort() throws IOException {
1170+
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1171+
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
1172+
try {
1173+
archive.parseExtendedHeader(toByteBuffer(0x00, 0x00), entryBuilder);
1174+
fail("Expected ArchiveException for invalid extended header length");
1175+
} catch (ArchiveException e) {
1176+
assertEquals("Invalid extended header length", e.getMessage());
1177+
}
1178+
}
1179+
}
1180+
11681181
@Test
11691182
void testParseExtendedHeaderCommon() throws IOException {
11701183
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1184+
// Valid
11711185
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
11721186
archive.parseExtendedHeader(toByteBuffer(0x00, 0x22, 0x33, 0x00, 0x00), entryBuilder);
11731187
assertEquals(0x3322, entryBuilder.get().getHeaderCrc());
1188+
1189+
// Invalid length
1190+
try {
1191+
archive.parseExtendedHeader(toByteBuffer(0x00, 0x22, 0x00, 0x00), entryBuilder);
1192+
fail("Expected ArchiveException for invalid extended header length");
1193+
} catch (ArchiveException e) {
1194+
assertEquals("Invalid extended header length", e.getMessage());
1195+
}
11741196
}
11751197
}
11761198

11771199
@Test
11781200
void testParseExtendedHeaderFilename() throws IOException {
11791201
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1202+
// Valid
11801203
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
11811204
archive.parseExtendedHeader(toByteBuffer(0x01, 't', 'e', 's', 't', '.', 't', 'x', 't', 0x00, 0x00), entryBuilder);
11821205
assertEquals("test.txt", entryBuilder.get().getName());
1206+
1207+
// Invalid length
1208+
try {
1209+
archive.parseExtendedHeader(toByteBuffer(0x01, 0x00), entryBuilder);
1210+
fail("Expected ArchiveException for invalid extended header length");
1211+
} catch (ArchiveException e) {
1212+
assertEquals("Invalid extended header length", e.getMessage());
1213+
}
11831214
}
11841215
}
11851216

@@ -1190,9 +1221,18 @@ void testParseExtendedHeaderDirectoryName() throws IOException {
11901221
.setFileSeparatorChar('/')
11911222
.get()) {
11921223

1224+
// Valid
11931225
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
11941226
archive.parseExtendedHeader(toByteBuffer(0x02, 'd', 'i', 'r', '1', 0xff, 0x00, 0x00), entryBuilder);
11951227
assertEquals("dir1/", entryBuilder.get().getName());
1228+
1229+
// Invalid length
1230+
try {
1231+
archive.parseExtendedHeader(toByteBuffer(0x02, 0x00), entryBuilder);
1232+
fail("Expected ArchiveException for invalid extended header length");
1233+
} catch (ArchiveException e) {
1234+
assertEquals("Invalid extended header length", e.getMessage());
1235+
}
11961236
}
11971237
}
11981238

@@ -1228,44 +1268,86 @@ void testParseExtendedHeaderFilenameAndDirectoryName() throws IOException {
12281268
archive.parseExtendedHeader(toByteBuffer(0x02, 'd', 'i', 'r', '1', 0x00, 0x00), entryBuilder);
12291269
archive.parseExtendedHeader(toByteBuffer(0x01, 't', 'e', 's', 't', '.', 't', 'x', 't', 0x00, 0x00), entryBuilder);
12301270
assertEquals("dir1/test.txt", entryBuilder.get().getName());
1271+
1272+
// Test empty directory name, no trailing slash
1273+
entryBuilder = LhaArchiveEntry.builder();
1274+
archive.parseExtendedHeader(toByteBuffer(0x02, 0x00, 0x00), entryBuilder);
1275+
archive.parseExtendedHeader(toByteBuffer(0x01, 't', 'e', 's', 't', '.', 't', 'x', 't', 0x00, 0x00), entryBuilder);
1276+
assertEquals("test.txt", entryBuilder.get().getName());
12311277
}
12321278
}
12331279

12341280
@Test
12351281
void testParseExtendedHeaderUnixPermission() throws IOException {
12361282
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1283+
// Valid
12371284
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
12381285
archive.parseExtendedHeader(toByteBuffer(0x50, 0xa4, 0x81, 0x00, 0x00), entryBuilder);
12391286
assertEquals(0x81a4, entryBuilder.get().getUnixPermissionMode());
12401287
assertEquals(0100644, entryBuilder.get().getUnixPermissionMode());
1288+
1289+
// Invalid length
1290+
try {
1291+
archive.parseExtendedHeader(toByteBuffer(0x50, 0xa4, 0x00, 0x00), entryBuilder);
1292+
fail("Expected ArchiveException for invalid extended header length");
1293+
} catch (ArchiveException e) {
1294+
assertEquals("Invalid extended header length", e.getMessage());
1295+
}
12411296
}
12421297
}
12431298

12441299
@Test
12451300
void testParseExtendedHeaderUnixUidGid() throws IOException {
12461301
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1302+
// Valid
12471303
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
12481304
archive.parseExtendedHeader(toByteBuffer(0x51, 0x14, 0x00, 0xf5, 0x01, 0x00, 0x00), entryBuilder);
12491305
assertEquals(0x0014, entryBuilder.get().getUnixGroupId());
12501306
assertEquals(0x01f5, entryBuilder.get().getUnixUserId());
1307+
1308+
// Invalid length
1309+
try {
1310+
archive.parseExtendedHeader(toByteBuffer(0x51, 0x14, 0x00, 0xf5, 0x00, 0x00), entryBuilder);
1311+
fail("Expected ArchiveException for invalid extended header length");
1312+
} catch (ArchiveException e) {
1313+
assertEquals("Invalid extended header length", e.getMessage());
1314+
}
12511315
}
12521316
}
12531317

12541318
@Test
12551319
void testParseExtendedHeaderUnixTimestamp() throws IOException {
12561320
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1321+
// Valid
12571322
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
12581323
archive.parseExtendedHeader(toByteBuffer(0x54, 0x5c, 0x73, 0x9c, 0x68, 0x00, 0x00), entryBuilder);
12591324
assertEquals(0x689c735cL, entryBuilder.get().getLastModifiedDate().getTime() / 1000);
1325+
1326+
// Invalid length
1327+
try {
1328+
archive.parseExtendedHeader(toByteBuffer(0x54, 0x5c, 0x73, 0x9c, 0x00, 0x00), entryBuilder);
1329+
fail("Expected ArchiveException for invalid extended header length");
1330+
} catch (ArchiveException e) {
1331+
assertEquals("Invalid extended header length", e.getMessage());
1332+
}
12601333
}
12611334
}
12621335

12631336
@Test
12641337
void testParseExtendedHeaderMSdosFileAttributes() throws IOException {
12651338
try (LhaArchiveInputStream archive = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1339+
// Valid
12661340
final LhaArchiveEntry.Builder entryBuilder = LhaArchiveEntry.builder();
1267-
archive.parseExtendedHeader(toByteBuffer(0x40, 0x10, 0x00, 0x00), entryBuilder);
1341+
archive.parseExtendedHeader(toByteBuffer(0x40, 0x10, 0x00, 0x00, 0x00), entryBuilder);
12681342
assertEquals(0x10, entryBuilder.get().getMsdosFileAttributes());
1343+
1344+
// Invalid length
1345+
try {
1346+
archive.parseExtendedHeader(toByteBuffer(0x40, 0x10, 0x00, 0x00), entryBuilder);
1347+
fail("Expected ArchiveException for invalid extended header length");
1348+
} catch (ArchiveException e) {
1349+
assertEquals("Invalid extended header length", e.getMessage());
1350+
}
12691351
}
12701352
}
12711353

@@ -1373,6 +1455,9 @@ void testGetCompressionMethod() throws IOException {
13731455
@Test
13741456
void testGetPathnameUnixFileSeparatorCharDefaultEncoding() throws IOException, UnsupportedEncodingException {
13751457
try (LhaArchiveInputStream is = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).setFileSeparatorChar('/').get()) {
1458+
assertEquals("", getPathname(is));
1459+
assertEquals("", getPathname(is, 0xff));
1460+
assertEquals("a", getPathname(is, 'a'));
13761461
assertEquals("folder/", getPathname(is, 'f', 'o', 'l', 'd', 'e', 'r', 0xff));
13771462
assertEquals("folder/file.txt", getPathname(is, 'f', 'o', 'l', 'd', 'e', 'r', 0xff, 'f', 'i', 'l', 'e', '.', 't', 'x', 't'));
13781463
assertEquals("folder/file.txt", getPathname(is, 0xff, 'f', 'o', 'l', 'd', 'e', 'r', 0xff, 'f', 'i', 'l', 'e', '.', 't', 'x', 't'));
@@ -1411,6 +1496,18 @@ void testGetPathnameWindowsFileSeparatorCharDefaultEncoding() throws IOException
14111496
}
14121497
}
14131498

1499+
@Test
1500+
void testGetPathnameNegativeLength() throws IOException, UnsupportedEncodingException {
1501+
try (LhaArchiveInputStream is = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1502+
try {
1503+
is.getPathname(ByteBuffer.wrap(new byte[0]), -1);
1504+
fail("Expected ArchiveException when pathname length is negative");
1505+
} catch (ArchiveException e) {
1506+
assertEquals("Pathname length is negative", e.getMessage());
1507+
}
1508+
}
1509+
}
1510+
14141511
@Test
14151512
void testGetPathnameTooLong() throws IOException, UnsupportedEncodingException {
14161513
try (LhaArchiveInputStream is = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
@@ -1424,6 +1521,19 @@ void testGetPathnameTooLong() throws IOException, UnsupportedEncodingException {
14241521
}
14251522
}
14261523

1524+
@Test
1525+
void testGetPathnameInvalidLength() throws IOException, UnsupportedEncodingException {
1526+
try (LhaArchiveInputStream is = LhaArchiveInputStream.builder().setInputStream(newEmptyInputStream()).get()) {
1527+
try {
1528+
final byte[] pathname = new byte[] { 'a', 'b', 'c' };
1529+
is.getPathname(ByteBuffer.wrap(pathname), pathname.length + 1);
1530+
fail("Expected ArchiveException for invalid pathname length");
1531+
} catch (ArchiveException e) {
1532+
assertEquals("Invalid pathname length", e.getMessage());
1533+
}
1534+
}
1535+
}
1536+
14271537
@Test
14281538
void testGetPathnameWindowsFileSeparatorCharIso88591() throws IOException, UnsupportedEncodingException {
14291539
try (LhaArchiveInputStream is = LhaArchiveInputStream.builder()

0 commit comments

Comments
 (0)