Properly handle denormalized href
authorKevin Ottens <kevin.ottens@nextcloud.com>
Mon, 18 May 2020 17:22:41 +0000 (19:22 +0200)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Mon, 18 May 2020 17:33:34 +0000 (19:33 +0200)
In case of denormalized paths in the dav href (presence of . or .. in
the path) simple string startsWith comparison wasn't enough to know if
said href ended up in the right namespace. That's why we're now using
QUrl (pretending local file since we don't have a full URL in the href)
to normalize the path before comparison.

This could happen with broken proxies for instance where we would
wrongly validate the dav information resulting in potentially surprising
syncing and name collisions.

Signed-off-by: Kevin Ottens <kevin.ottens@nextcloud.com>
src/libsync/networkjobs.cpp
test/testxmlparse.cpp

index 885a89618751e0c09825044a53680aa2f8316949..20e00f1a9a07e4e2daa5ef0c3982e9a546d3177a 100644 (file)
@@ -209,7 +209,9 @@ bool LsColXMLParser::parse(const QByteArray &xml, QHash<QString, ExtraFolderInfo
             if (name == QLatin1String("href")) {
                 // We don't use URL encoding in our request URL (which is the expected path) (QNAM will do it for us)
                 // but the result will have URL encoding..
-                QString hrefString = QString::fromUtf8(QByteArray::fromPercentEncoding(reader.readElementText().toUtf8()));
+                QString hrefString = QUrl::fromLocalFile(QUrl::fromPercentEncoding(reader.readElementText().toUtf8()))
+                        .adjusted(QUrl::NormalizePathSegments)
+                        .path();
                 if (!hrefString.startsWith(expectedPath)) {
                     qCWarning(lcLsColJob) << "Invalid href" << hrefString << "expected starting with" << expectedPath;
                     return false;
index 79bdda724773a0c060b0d224b151d4f940f0857c..f246a3a4c9a24076210d2b9a12de57c7379cd296 100644 (file)
@@ -402,6 +402,146 @@ private slots:
         QVERIFY(!_success);
     }
 
+    void testParserDenormalizedPath() {
+        const QByteArray testXml = "<?xml version='1.0' encoding='utf-8'?>"
+              "<d:multistatus xmlns:d=\"DAV:\" xmlns:s=\"http://sabredav.org/ns\" xmlns:oc=\"http://owncloud.org/ns\">"
+              "<d:response>"
+              "<d:href>/oc/remote.php/webdav/sharefolder/</d:href>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:id>00004213ocobzus5kn6s</oc:id>"
+              "<oc:permissions>RDNVCK</oc:permissions>"
+              "<oc:size>121780</oc:size>"
+              "<d:getetag>\"5527beb0400b0\"</d:getetag>"
+              "<d:resourcetype>"
+              "<d:collection/>"
+              "</d:resourcetype>"
+              "<d:getlastmodified>Fri, 06 Feb 2015 13:49:55 GMT</d:getlastmodified>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 200 OK</d:status>"
+              "</d:propstat>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<d:getcontentlength/>"
+              "<oc:downloadURL/>"
+              "<oc:dDC/>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 404 Not Found</d:status>"
+              "</d:propstat>"
+              "</d:response>"
+              "<d:response>"
+              "<d:href>/oc/remote.php/webdav/sharefolder/../sharefolder/quitte.pdf</d:href>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:id>00004215ocobzus5kn6s</oc:id>"
+              "<oc:permissions>RDNVW</oc:permissions>"
+              "<d:getetag>\"2fa2f0d9ed49ea0c3e409d49e652dea0\"</d:getetag>"
+              "<d:resourcetype/>"
+              "<d:getlastmodified>Fri, 06 Feb 2015 13:49:55 GMT</d:getlastmodified>"
+              "<d:getcontentlength>121780</d:getcontentlength>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 200 OK</d:status>"
+              "</d:propstat>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:downloadURL/>"
+              "<oc:dDC/>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 404 Not Found</d:status>"
+              "</d:propstat>"
+              "</d:response>"
+              "</d:multistatus>";
+
+
+        LsColXMLParser parser;
+
+        connect( &parser, SIGNAL(directoryListingSubfolders(const QStringList&)),
+                 this, SLOT(slotDirectoryListingSubFolders(const QStringList&)) );
+        connect( &parser, SIGNAL(directoryListingIterated(const QString&, const QMap<QString,QString>&)),
+                 this, SLOT(slotDirectoryListingIterated(const QString&, const QMap<QString,QString>&)) );
+        connect( &parser, SIGNAL(finishedWithoutError()),
+                 this, SLOT(slotFinishedSuccessfully()) );
+
+        QHash <QString, ExtraFolderInfo> sizes;
+        QVERIFY(parser.parse( testXml, &sizes, "/oc/remote.php/webdav/sharefolder" ));
+
+        QVERIFY(_success);
+        QCOMPARE(sizes.size(), 1 ); // Quota info in the XML
+
+        QVERIFY(_items.contains("/oc/remote.php/webdav/sharefolder/quitte.pdf"));
+        QVERIFY(_items.contains("/oc/remote.php/webdav/sharefolder"));
+        QVERIFY(_items.size() == 2 );
+
+        QVERIFY(_subdirs.contains("/oc/remote.php/webdav/sharefolder/"));
+        QVERIFY(_subdirs.size() == 1);
+    }
+
+    void testParserDenormalizedPathOutsideNamespace() {
+        const QByteArray testXml = "<?xml version='1.0' encoding='utf-8'?>"
+              "<d:multistatus xmlns:d=\"DAV:\" xmlns:s=\"http://sabredav.org/ns\" xmlns:oc=\"http://owncloud.org/ns\">"
+              "<d:response>"
+              "<d:href>/oc/remote.php/webdav/sharefolder/</d:href>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:id>00004213ocobzus5kn6s</oc:id>"
+              "<oc:permissions>RDNVCK</oc:permissions>"
+              "<oc:size>121780</oc:size>"
+              "<d:getetag>\"5527beb0400b0\"</d:getetag>"
+              "<d:resourcetype>"
+              "<d:collection/>"
+              "</d:resourcetype>"
+              "<d:getlastmodified>Fri, 06 Feb 2015 13:49:55 GMT</d:getlastmodified>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 200 OK</d:status>"
+              "</d:propstat>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<d:getcontentlength/>"
+              "<oc:downloadURL/>"
+              "<oc:dDC/>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 404 Not Found</d:status>"
+              "</d:propstat>"
+              "</d:response>"
+              "<d:response>"
+              "<d:href>/oc/remote.php/webdav/sharefolder/../quitte.pdf</d:href>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:id>00004215ocobzus5kn6s</oc:id>"
+              "<oc:permissions>RDNVW</oc:permissions>"
+              "<d:getetag>\"2fa2f0d9ed49ea0c3e409d49e652dea0\"</d:getetag>"
+              "<d:resourcetype/>"
+              "<d:getlastmodified>Fri, 06 Feb 2015 13:49:55 GMT</d:getlastmodified>"
+              "<d:getcontentlength>121780</d:getcontentlength>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 200 OK</d:status>"
+              "</d:propstat>"
+              "<d:propstat>"
+              "<d:prop>"
+              "<oc:downloadURL/>"
+              "<oc:dDC/>"
+              "</d:prop>"
+              "<d:status>HTTP/1.1 404 Not Found</d:status>"
+              "</d:propstat>"
+              "</d:response>"
+              "</d:multistatus>";
+
+
+        LsColXMLParser parser;
+
+        connect( &parser, SIGNAL(directoryListingSubfolders(const QStringList&)),
+                 this, SLOT(slotDirectoryListingSubFolders(const QStringList&)) );
+        connect( &parser, SIGNAL(directoryListingIterated(const QString&, const QMap<QString,QString>&)),
+                 this, SLOT(slotDirectoryListingIterated(const QString&, const QMap<QString,QString>&)) );
+        connect( &parser, SIGNAL(finishedWithoutError()),
+                 this, SLOT(slotFinishedSuccessfully()) );
+
+        QHash <QString, ExtraFolderInfo> sizes;
+        QVERIFY(!parser.parse( testXml, &sizes, "/oc/remote.php/webdav/sharefolder" ));
+
+        QVERIFY(!_success);
+    }
+
     void testHrefUrlEncoding() {
         const QByteArray testXml = "<?xml version='1.0' encoding='utf-8'?>"
               "<d:multistatus xmlns:d=\"DAV:\" xmlns:s=\"http://sabredav.org/ns\" xmlns:oc=\"http://owncloud.org/ns\">"