commit edb0799c352af02f04e6f50b85412b4118cc86ac Author: Rolf Eike Beer <kde@opensource.sf-tec.de> Date: Thu Jul 28 00:06:48 2011 +0200 fix filename security check being omitted on parse error in HTTP header A header like this: Content-Disposition: attachment; filename="/home/eike/.gnupg/gpg.conf"; foo="bar; foo="baz" would not have the path from the filename stripped because of the later parse error. This adds a unit test for this and some other cornercases. CCBUG:278643 backport of 54e8eded22c4af61f609b6184cc0293df407a2f9 diff --git a/kioslave/http/parsinghelpers.cpp b/kioslave/http/parsinghelpers.cpp index d8ed06a..2102c40 100644 --- a/kioslave/http/parsinghelpers.cpp +++ b/kioslave/http/parsinghelpers.cpp @@ -23,6 +23,7 @@ #include <QUrl> #include <kcodecs.h> +#include <kdebug.h> // Advance *pos beyond spaces / tabs static void skipSpace(const char input[], int *pos, int end) @@ -408,7 +409,7 @@ static QString extractMaybeQuotedUntil(const QString &str, int &pos) } } -static QMap<QString, QString> contentDispositionParser(const QString &disposition) +static QMap<QString, QString> contentDispositionParserInternal(const QString &disposition) { kDebug(7113) << "disposition: " << disposition; int pos = 0; @@ -430,7 +431,7 @@ static QMap<QString, QString> contentDispositionParser(const QString &dispositio if( key.isEmpty() ) { // parse error in this key: do not parse more, but add up // everything we already got - kDebug(7113) << "parse error, abort parsing"; + kDebug(7113) << "parse error in key, abort parsing"; break; } @@ -442,7 +443,7 @@ static QMap<QString, QString> contentDispositionParser(const QString &dispositio if( val.isEmpty() ) { if( pos == -1 ) { - kDebug(7113) << "parse error, abort parsing"; + kDebug(7113) << "parse error in value, abort parsing"; break; } continue; @@ -557,6 +558,13 @@ static QMap<QString, QString> contentDispositionParser(const QString &dispositio } } + return parameters; +} + +static QMap<QString, QString> contentDispositionParser(const QString &disposition) +{ + QMap<QString, QString> parameters = contentDispositionParserInternal(disposition); + const QLatin1String fn("filename"); if( parameters.contains(fn) ) { // Content-Disposition is not allowed to dictate directory diff --git a/kioslave/tests/httpheaderdispositiontest.cpp b/kioslave/tests/httpheaderdispositiontest.cpp index 5191e9f..3859d43 100644 --- a/kioslave/tests/httpheaderdispositiontest.cpp +++ b/kioslave/tests/httpheaderdispositiontest.cpp @@ -193,6 +193,17 @@ static const struct { "filename\tfoo-ä-€.html" }, // missing closing quote, so parameter is broken { "attachment; filename=\"bar", + "type\tattachment" }, +// we ignore any path given in the header and use only the filename + { "attachment; filename=\"/etc/shadow\"", + "type\tattachment\n" + "filename\tshadow" }, +// we ignore any path given in the header and use only the filename even if there is an error later + { "attachment; filename=\"/etc/shadow\"; foo=\"baz\"; foo=\"bar\"", + "type\tattachment\n" + "filename\tshadow" }, +// duplicate keys must be ignored + { "attachment; filename=\"bar\"; filename*0=\"foo.\"; filename*1=\"html\"", "type\tattachment" } };