From fe1b406f352d893a30f7abd0e2213f5e40a375ef Mon Sep 17 00:00:00 2001 From: Raul Metsma Date: Fri, 15 May 2026 13:31:07 +0300 Subject: [PATCH] Be more strict with datafile name IB-8936 Signed-off-by: Raul Metsma --- src/ASiC_E.cpp | 12 ++++++++---- src/ASiC_S.cpp | 6 +----- src/ASiContainer.cpp | 33 ++++++++++++++++++++++++++++---- src/ASiContainer.h | 4 +++- test/data/asice-path.asice | Bin 0 -> 735 bytes test/data/asice-relative.asice | Bin 0 -> 744 bytes test/data/asics-subfolder.asics | Bin 0 -> 386 bytes test/libdigidocpp_boost.cpp | 17 ++++++++++++++++ 8 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 test/data/asice-path.asice create mode 100644 test/data/asice-relative.asice create mode 100644 test/data/asics-subfolder.asics diff --git a/src/ASiC_E.cpp b/src/ASiC_E.cpp index 33a83da01..6b6f944fb 100644 --- a/src/ASiC_E.cpp +++ b/src/ASiC_E.cpp @@ -57,7 +57,8 @@ ASiC_E::ASiC_E(const string &path, bool create) try return; auto z = load(true, {MIMETYPE_ASIC_E, MIMETYPE_ADOC}); auto doc = XMLDocument::open(z.read("META-INF/manifest.xml"), {"manifest", MANIFEST_NS}); - doc.validateSchema(File::path(Conf::instance()->xsdPath(), "OpenDocument_manifest_v1_2.xsd")); + static const XMLSchema schema(File::path(Conf::instance()->xsdPath(), "OpenDocument_manifest_v1_2.xsd")); + doc.validateSchema(schema); set manifestFiles; bool mimeFound = false; @@ -67,6 +68,8 @@ ASiC_E::ASiC_E(const string &path, bool create) try auto media_type = file[{"media-type", MANIFEST_NS}]; DEBUG("full_path = '%.*s', media_type = '%.*s'", STR_VIEW_FMT(full_path), STR_VIEW_FMT(media_type)); + if(full_path.empty()) + THROW("Manifest file entry full-path is empty."); // ODF does not specify that mimetype should be first in manifest if(full_path == "/") { @@ -80,13 +83,14 @@ ASiC_E::ASiC_E(const string &path, bool create) try if(full_path.back() == '/') // Skip Directory entries continue; - if(const auto &[pos, inserted] = manifestFiles.insert(full_path); !inserted) + if(const auto &[_, inserted] = manifestFiles.insert(full_path); !inserted) THROW("Manifest multiple entries defined for file '%.*s'.", STR_VIEW_FMT(full_path)); + validateDataFilePath(full_path); if(mediaType() == MIMETYPE_ADOC && (full_path.starts_with("META-INF/") || full_path.starts_with("metadata/"))) d->metadata.push_back(new DataFilePrivate(z, string(full_path), string(media_type))); else - addDataFilePrivate(new DataFilePrivate(z, string(full_path), string(media_type))); + addDataFilePrivate(z, full_path, media_type); } if(!mimeFound) THROW("Manifest is missing mediatype file entry."); @@ -98,7 +102,7 @@ ASiC_E::ASiC_E(const string &path, bool create) try * 6.2.2 Contents of Container * 3) The root element of each "*signatures*.xml" content shall be either: */ - if(file.starts_with("META-INF/") && file.contains("signatures")) + if(file.starts_with("META-INF/") && file.contains("signatures") && file.ends_with(".xml")) { manifestFiles.erase(file); try diff --git a/src/ASiC_S.cpp b/src/ASiC_S.cpp index 770163aba..35e40b478 100644 --- a/src/ASiC_S.cpp +++ b/src/ASiC_S.cpp @@ -19,7 +19,6 @@ #include "ASiC_S.h" -#include "DataFile_p.h" #include "SignatureTST.h" #include "SignatureXAdES_LTA.h" #include "crypto/Signer.h" @@ -71,10 +70,7 @@ ASiC_S::ASiC_S(const string &path, bool create) else if(!dataFiles().empty()) THROW("Can not add document to ASiC-S container which already contains a document."); else - { - addDataFileChecks(file, "application/octet-stream"); - addDataFilePrivate(new DataFilePrivate(z, file, "application/octet-stream")); - } + addDataFilePrivate(z, file, "application/octet-stream"); } if(foundTimestamp && !foundManifest) { diff --git a/src/ASiContainer.cpp b/src/ASiContainer.cpp index 15b3ab898..7b5d61a02 100644 --- a/src/ASiContainer.cpp +++ b/src/ASiContainer.cpp @@ -169,26 +169,51 @@ void ASiContainer::addDataFile(const string &path, const string &mediaType) void ASiContainer::addDataFile(unique_ptr is, const string &fileName, const string &mediaType) { addDataFileChecks(fileName, mediaType); - if(fileName.find_last_of("/\\") != string::npos) - THROW("Document file '%s' cannot contain directory path.", fileName.c_str()); d->documents.push_back(new DataFilePrivate(std::move(is), fileName, mediaType)); } +void ASiContainer::validateDataFileName(string_view fileName) +{ + if(fileName.empty() || fileName == "." || fileName == ".." || + fileName.find_first_of("/\\") != string_view::npos) + THROW("Document file '%.*s' cannot contain directory path.", STR_VIEW_FMT(fileName)); +} + +void ASiContainer::validateDataFilePath(string_view fileName) +{ + if(fileName.empty() || fileName.front() == '/' || fileName.back() == '/' || + fileName.find('\\') != string_view::npos) + THROW("Document file '%.*s' contains invalid path.", STR_VIEW_FMT(fileName)); + + for(size_t pos = 0; pos < fileName.size();) + { + size_t next = fileName.find('/', pos); + string_view segment = fileName.substr(pos, next == string_view::npos ? next : next - pos); + if(segment.empty() || segment == "." || segment == "..") + THROW("Document file '%.*s' contains invalid path.", STR_VIEW_FMT(fileName)); + if(next == string_view::npos) + break; + pos = next + 1; + } +} + void ASiContainer::addDataFileChecks(const string &fileName, const string &mediaType) { if(!d->signatures.empty()) THROW("Can not add document to container which has signatures, remove all signatures before adding new document."); if(fileName == "mimetype") THROW("mimetype is reserved file."); + validateDataFileName(fileName); if(any_of(d->documents.cbegin(), d->documents.cend(), [&](DataFile *file) { return fileName == file->fileName(); })) THROW("Document with same file name '%s' already exists.", fileName.c_str()); if(mediaType.find('/') == string::npos) THROW("MediaType does not meet format requirements (RFC2045, section 5.1) '%s'.", mediaType.c_str()); } -void ASiContainer::addDataFilePrivate(DataFile *dataFile) +void ASiContainer::addDataFilePrivate(const ZipSerialize &z, string_view filename, string_view mediatype) { - d->documents.push_back(dataFile); + validateDataFilePath(filename); + d->documents.push_back(new DataFilePrivate(z, string(filename), string(mediatype))); } /** diff --git a/src/ASiContainer.h b/src/ASiContainer.h index e92674da7..291bf362f 100644 --- a/src/ASiContainer.h +++ b/src/ASiContainer.h @@ -63,13 +63,15 @@ namespace digidoc ASiContainer(const std::string &path, std::string_view mimetype); virtual void addDataFileChecks(const std::string &path, const std::string &mediaType); - void addDataFilePrivate(DataFile *dataFile); + void addDataFilePrivate(const ZipSerialize &z, std::string_view filename, std::string_view mediatype); Signature* addSignature(std::unique_ptr &&signature); virtual void canSave() = 0; XMLDocument createManifest() const; ZipSerialize load(bool requireMimetype, const std::set &supported); virtual void save(const ZipSerialize &s) = 0; void deleteSignature(Signature* s); + static void validateDataFilePath(std::string_view fileName); + static void validateDataFileName(std::string_view fileName); const ZipSerialize::Properties& zproperty(std::string_view file) const; diff --git a/test/data/asice-path.asice b/test/data/asice-path.asice new file mode 100644 index 0000000000000000000000000000000000000000..a7167b73af011becf7aa55c1e539b7a20ec8636b GIT binary patch literal 735 zcma)4y-ve05KezUj0lyP;1wmIOvq|lF|-n@KpDydSjmN2vYjY)tCUAzVB{^Bc@lQs zp%WYDN=e%Y1dh%*Tlan6>HJ00PPgYcN6uzFpMQ<*Fc;b1qM^e6iyv$pUW8)M_?#8nl@AiHYNTzrS+W4gugAs1-5(< z#03>+30IFHd6-SSYcd)RDhRC-7hy`*$B--vMKf$gQ^h1`YGRt@ppq><`n=XX#a%Scs3CQ+ZLc*D6hZK@O#zcbuOQe}dpa=|(tM8lA z{Gw=}G(j#HSC+h2nW+SG zYXoLS!~ws26FS-=`D)*sy*510#R-r+3@3NN{o{yaOo@~mi|3i_4Pf(UyfDBSqAtEd zK38#`2=yGIr^zU|MT35?gwQha0j5lK4AHbuG>29URV?F%+L-2=t0a%78CRBW*VC0y zNk%vHuhE8DDkKj8T`c^nHl0ctXl7qRM9}&=G&@Pc4WEKID6 z?cDAWwR~=gmP{zeUk$k$QGfe&;^p$A1+U`b7-EM|!Yqr%jf0N2-x+}1Sfx+usJa!_ ikRhZyjZvql_q(R}27m47J{q?VT~QOhPxzl6xYIAmKFI|D literal 0 HcmV?d00001 diff --git a/test/data/asics-subfolder.asics b/test/data/asics-subfolder.asics new file mode 100644 index 0000000000000000000000000000000000000000..6a338bd9de1c5c459f5dcce27eae8b1abfa62060 GIT binary patch literal 386 zcmWIWW@Zs#fB;2?70!{X=^l9E`Gh^oWx z*|nvdK$RdY2E@LuA&$D9es20DK>LeJ5_1dmN{UP1oB(e|CJ|=bt_GR_1`UiL3g!o7 uUD%un(Zj&dz~~BOqB|O0H@cG%y32t~>>dd4W@Q7ZV+O(signatures().at(0)->signingCertificate().subjectName("CN"), "MÖLDER,HUGO MARTIN,38910239121"); BOOST_CHECK_THROW(d->signatures().at(0)->validate(), Exception); } + +BOOST_AUTO_TEST_CASE(manifest_data_file_paths_are_supported) +{ + auto d = Container::openPtr("asice-path.asice"); + BOOST_REQUIRE_EQUAL(d->dataFiles().size(), 1U); + BOOST_CHECK_EQUAL(d->dataFiles().front()->fileName(), "folder/test1.txt"); +} + +BOOST_AUTO_TEST_CASE(manifest_data_file_relative_paths_are_rejected) +{ + BOOST_CHECK_THROW(Container::openPtr("asice-relative.asice"), Exception); +} BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE(ASiCSTestSuite) @@ -611,6 +623,11 @@ BOOST_AUTO_TEST_CASE(OpenInvalidMimetypeContainer) { BOOST_CHECK_THROW(Container::openPtr("test-invalid.asics"), Exception); } + +BOOST_AUTO_TEST_CASE(OpenASiCSContainerWithSubfolderDataObject) +{ + BOOST_CHECK_THROW(Container::openPtr("asics-subfolder.asics"), Exception); +} BOOST_AUTO_TEST_SUITE_END() BOOST_AUTO_TEST_SUITE(ExtendValiditySuite)