Skip to content

Commit 1642b4c

Browse files
committed
Improve JVM file handling, taking values from CEN, fix failing to read data from last local entry
1 parent 64635f7 commit 1642b4c

File tree

2 files changed

+51
-16
lines changed

2 files changed

+51
-16
lines changed

src/main/java/software/coley/llzip/format/model/JvmLocalFileHeader.java

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@
1515
*/
1616
public class JvmLocalFileHeader extends LocalFileHeader {
1717
private final NavigableSet<Long> offsets;
18-
private long offset;
19-
private boolean foundPk;
18+
private long dataOffsetStart;
19+
private long dataOffsetEnd;
20+
private boolean foundData;
2021
private ByteData data;
2122

2223
/**
@@ -33,29 +34,53 @@ public void read(ByteData data, long offset) {
3334

3435
// JVM file data reading does NOT use the compressed/uncompressed fields.
3536
// Instead, it scans data until the next header.
36-
offset += MIN_FIXED_SIZE + getFileNameLength() + getExtraFieldLength();
37-
this.offset = offset;
38-
Long nextOffset = offsets.ceiling(offset);
39-
if (nextOffset != null) {
40-
setFileData(data.slice(offset, nextOffset));
41-
foundPk = true;
37+
long dataOffsetStart = offset + MIN_FIXED_SIZE + getFileNameLength() + getExtraFieldLength();
38+
Long dataOffsetEnd = offsets.ceiling(dataOffsetStart);
39+
this.dataOffsetStart = dataOffsetStart;
40+
this.dataOffsetEnd = dataOffsetEnd == null ? -1 : dataOffsetEnd;
41+
if (dataOffsetEnd != null) {
42+
// Valid data range found
43+
setFileData(data.slice(dataOffsetStart, dataOffsetEnd));
44+
foundData = true;
4245
} else {
46+
// Keep data reference to attempt restoration with later when linking to the CEN.
4347
this.data = data;
4448
}
4549
}
4650

4751
@Override
4852
public void link(CentralDirectoryFileHeader directoryFileHeader) {
4953
super.link(directoryFileHeader);
50-
if (!foundPk) {
54+
55+
// JVM trusts central directory file header contents over local
56+
// - Using fields as this maintains the lazy model
57+
compressionMethod = directoryFileHeader.compressionMethod;
58+
compressedSize = directoryFileHeader.compressedSize;
59+
uncompressedSize = directoryFileHeader.uncompressedSize;
60+
fileName = directoryFileHeader.fileName;
61+
generalPurposeBitFlag = directoryFileHeader.generalPurposeBitFlag;
62+
crc32 = directoryFileHeader.crc32;
63+
lastModFileDate = directoryFileHeader.lastModFileDate;
64+
lastModFileTime = directoryFileHeader.lastModFileTime;
65+
66+
// Update file data with new compressed/uncompressed size if it was not able to be found previously
67+
// with only the local data available.
68+
if (!foundData) {
69+
// Extract updated length from CEN values
5170
long fileDataLength;
5271
if (getCompressionMethod() == ZipCompressions.STORED) {
53-
fileDataLength = directoryFileHeader.getUncompressedSize() & 0xFFFFFFFFL;
72+
fileDataLength = getUncompressedSize() & 0xFFFFFFFFL;
5473
} else {
55-
fileDataLength = directoryFileHeader.getCompressedSize() & 0xFFFFFFFFL;
74+
fileDataLength = getCompressedSize() & 0xFFFFFFFFL;
75+
}
76+
77+
// Only allow lengths that are within a sensible range.
78+
// Data should not be overflowing into adjacent header entries.
79+
// - If it is, the data here is likely intentionally tampered with to screw with parsers
80+
if (fileDataLength + offset < dataOffsetEnd) {
81+
setFileData(data.sliceOf(dataOffsetStart, fileDataLength));
82+
data = null;
5683
}
57-
setFileData(data.sliceOf(offset, fileDataLength));
58-
data = null;
5984
}
6085
}
6186
}

src/main/java/software/coley/llzip/format/read/JvmZipReaderStrategy.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,29 @@ else if (data.getInt(jvmBaseFileOffset) == ZipPatterns.CENTRAL_DIRECTORY_FILE_HE
111111
// Read local files
112112
// - Set to prevent duplicate file header entries for the same offset
113113
Set<Long> offsets = new HashSet<>();
114-
TreeSet<Long> lfhOffsets = new TreeSet<>();
114+
TreeSet<Long> entryOffsets = new TreeSet<>();
115+
long earliestCdfh = Long.MAX_VALUE;
115116
for (CentralDirectoryFileHeader directory : zip.getCentralDirectories()) {
117+
// Update earliest central offset
118+
if (directory.offset() < earliestCdfh)
119+
earliestCdfh = directory.offset();
120+
121+
// Add associated local file header offset
116122
long offset = jvmBaseFileOffset + directory.getRelativeOffsetOfLocalHeader();
117123
if (data.getInt(offset) == ZipPatterns.LOCAL_FILE_HEADER_QUAD) {
118-
lfhOffsets.add(offset);
124+
entryOffsets.add(offset);
119125
}
120126
}
127+
// Add the earliest central directory offset, which serves as the upper bound to search against for the
128+
// last local file header entry's file data contents.
129+
entryOffsets.add(earliestCdfh);
130+
121131
for (CentralDirectoryFileHeader directory : zip.getCentralDirectories()) {
122132
long relative = directory.getRelativeOffsetOfLocalHeader();
123133
long offset = jvmBaseFileOffset + relative;
124134
if (!offsets.contains(offset) && data.getInt(offset) == ZipPatterns.LOCAL_FILE_HEADER_QUAD) {
125135
try {
126-
JvmLocalFileHeader file = new JvmLocalFileHeader(lfhOffsets);
136+
JvmLocalFileHeader file = new JvmLocalFileHeader(entryOffsets);
127137
file.read(data, offset);
128138
zip.getParts().add(file);
129139
directory.link(file);

0 commit comments

Comments
 (0)