Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correctly test invalid offsets in clusters. #895

Merged
merged 3 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/zim/zim.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ namespace zim
*/
CLUSTER_PTRS,

/**
* Checks that offsets inside each clusters are valid.
*/
CLUSTERS_OFFSETS,

/**
* Checks that mime-type values in dirents are valid.
*/
Expand Down
4 changes: 3 additions & 1 deletion src/cluster.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,9 @@ getClusterReader(const Reader& zimReader, offset_t offset, Cluster::Compression*
while (--n_offset)
{
OFFSET_TYPE new_offset = seqReader.read<OFFSET_TYPE>();
ASSERT(new_offset, >=, offset);
if (new_offset < offset) {
throw zim::ZimFileFormatError("Error parsing cluster. Offsets are not ordered.");
}

m_blobOffsets.push_back(offset_t(new_offset));
offset = new_offset;
Expand Down
16 changes: 16 additions & 0 deletions src/fileimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,7 @@ class Grouping
case IntegrityCheck::DIRENT_ORDER: return FileImpl::checkDirentOrder();
case IntegrityCheck::TITLE_INDEX: return FileImpl::checkTitleIndex();
case IntegrityCheck::CLUSTER_PTRS: return FileImpl::checkClusterPtrs();
case IntegrityCheck::CLUSTERS_OFFSETS: return FileImpl::checkClusters();
case IntegrityCheck::DIRENT_MIMETYPES: return FileImpl::checkDirentMimeTypes();
case IntegrityCheck::COUNT: ASSERT("shouldn't have reached here", ==, "");
}
Expand Down Expand Up @@ -676,6 +677,21 @@ class Grouping
return true;
}

bool FileImpl::checkClusters() {
const cluster_index_type clusterCount = getCountClusters().v;
for ( cluster_index_type i = 0; i < clusterCount; ++i )
{
// Force a read of each clusters (which will throw ZimFileFormatError in case of error)
try {
readCluster(cluster_index_t(i));
} catch (ZimFileFormatError& e) {
std::cerr << e.what() << std::endl;
return false;
}
}
return true;
}

bool FileImpl::checkClusterPtrs() {
const cluster_index_type clusterCount = getCountClusters().v;
const offset_t validClusterRangeStart(80); // XXX: really???
Expand Down
1 change: 1 addition & 0 deletions src/fileimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ namespace zim
bool checkDirentOrder();
bool checkTitleIndex();
bool checkClusterPtrs();
bool checkClusters();
bool checkDirentMimeTypes();
};

Expand Down
5 changes: 5 additions & 0 deletions test/archive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,11 @@ TEST(ZimArchive, validate)
"Invalid cluster pointer\n"
);

TEST_BROKEN_ZIM_NAME(
kelson42 marked this conversation as resolved.
Show resolved Hide resolved
"invalid.offset_in_cluster.zim",
"Error parsing cluster. Offsets are not ordered.\n"
)


for(auto& testfile: getDataFilePath("invalid.nonsorted_dirent_table.zim")) {
std::string expected;
Expand Down
Loading