Be more strict with datafile name

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<string_view> 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

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)
     {

src/ASiContainer.cpp

@@ -169,26 +169,51 @@ void ASiContainer::addDataFile(const string &path, const string &mediaType)
 void ASiContainer::addDataFile(unique_ptr<istream> 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)));
 }
 
 /**

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> &&signature);
           virtual void canSave() = 0;
           XMLDocument createManifest() const;
           ZipSerialize load(bool requireMimetype, const std::set<std::string_view> &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;
 

test/libdigidocpp_boost.cpp

@@ -524,6 +524,18 @@ BOOST_AUTO_TEST_CASE(key_substitution_detected)
     BOOST_CHECK_EQUAL(d->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)