Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2024-10-01 01:26:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

Fix subdomain matching

Browse Source
This commit is contained in:
varjolintu 2019-11-12 22:38:20 +02:00 committed by Jonathan White
parent a590289900
commit e2c95f75f1
4 changed files with 167 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
src/browser/BrowserService.cpp
View File

@ -380,7 +380,7 @@ QJsonArray BrowserService::findMatchingEntries(const QString& id,
// Check entries for authorization
QList<Entry*> pwEntriesToConfirm;
QList<Entry*> pwEntries;
for (auto* entry : searchEntries(url, keyList)) {
for (auto* entry : searchEntries(url, submitUrl, keyList)) {
if (entry->customData()->contains(BrowserService::OPTION_HIDE_ENTRY)
&& entry->customData()->value(BrowserService::OPTION_HIDE_ENTRY) == "true") {
continue;
@ -583,7 +583,7 @@ BrowserService::ReturnValue BrowserService::updateEntry(const QString& id,
}
QList<Entry*>
BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString& hostname, const QString& url)
BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString& url, const QString& submitUrl)
{
QList<Entry*> entries;
auto* rootGroup = db->rootGroup();
@ -601,20 +601,18 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
continue;
}
auto domain = baseDomain(hostname);
// Search for additional URL's starting with KP2A_URL
if (entry->attributes()->keys().contains(ADDITIONAL_URL)) {
for (const auto& key : entry->attributes()->keys()) {
if (key.startsWith(ADDITIONAL_URL)
&& handleURL(entry->attributes()->value(key), domain, url)) {
&& handleURL(entry->attributes()->value(key), url, submitUrl)) {
entries.append(entry);
continue;
}
}
}
if (!handleURL(entry->url(), domain, url)) {
if (!handleURL(entry->url(), url, submitUrl)) {
continue;
}
@ -625,7 +623,7 @@ BrowserService::searchEntries(const QSharedPointer<Database>& db, const QString&
return entries;
}
QList<Entry*> BrowserService::searchEntries(const QString& url, const StringPairList& keyList)
QList<Entry*> BrowserService::searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList)
{
// Check if database is connected with KeePassXC-Browser
auto databaseConnected = [&](const QSharedPointer<Database>& db) {
@ -662,7 +660,7 @@ QList<Entry*> BrowserService::searchEntries(const QString& url, const StringPair
QList<Entry*> entries;
do {
for (const auto& db : databases) {
entries << searchEntries(db, hostname, url);
entries << searchEntries(db, url, submitUrl);
}
} while (entries.isEmpty() && removeFirstDomain(hostname));
@ -1000,7 +998,7 @@ bool BrowserService::removeFirstDomain(QString& hostname)
return false;
}
bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname, const QString& url)
bool BrowserService::handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl)
{
if (entryUrl.isEmpty()) {
return false;
@ -1017,31 +1015,35 @@ bool BrowserService::handleURL(const QString& entryUrl, const QString& hostname,
}
}
// Make a direct compare if a local file is used
if (url.contains("file://")) {
return entryUrl == submitUrl;
}
// URL host validation fails
if (browserSettings()->matchUrlScheme() && entryQUrl.host().isEmpty()) {
if (entryQUrl.host().isEmpty()) {
return false;
}
// Match port, if used
QUrl qUrl(url);
if (entryQUrl.port() > 0 && entryQUrl.port() != qUrl.port()) {
QUrl siteQUrl(url);
if (entryQUrl.port() > 0 && entryQUrl.port() != siteQUrl.port()) {
return false;
}
// Match scheme
if (browserSettings()->matchUrlScheme() && !entryQUrl.scheme().isEmpty() && entryQUrl.scheme().compare(qUrl.scheme()) != 0) {
if (browserSettings()->matchUrlScheme() && !entryQUrl.scheme().isEmpty() && entryQUrl.scheme().compare(siteQUrl.scheme()) != 0) {
return false;
}
// Check for illegal characters
QRegularExpression re("[<>\\^`{|}]");
auto match = re.match(entryUrl);
if (match.hasMatch()) {
if (re.match(entryUrl).hasMatch()) {
return false;
}
// Filter to match hostname in URL field
if (entryQUrl.host().endsWith(hostname)) {
if (siteQUrl.host().endsWith(entryQUrl.host())) {
return true;
}

6
src/browser/BrowserService.h
View File

@ -63,8 +63,8 @@ public:
const QString& group,
const QString& groupUuid,
const QSharedPointer<Database>& selectedDb = {});
QList<Entry*> searchEntries(const QSharedPointer<Database>& db, const QString& hostname, const QString& url);
QList<Entry*> searchEntries(const QString& url, const StringPairList& keyList);
QList<Entry*> searchEntries(const QSharedPointer<Database>& db, const QString& url, const QString& submitUrl);
QList<Entry*> searchEntries(const QString& url, const QString& submitUrl, const StringPairList& keyList);
void convertAttributesToCustomData(const QSharedPointer<Database>& currentDb = {});
public:
@ -130,7 +130,7 @@ private:
sortPriority(const Entry* entry, const QString& host, const QString& submitUrl, const QString& baseSubmitUrl) const;
bool schemeFound(const QString& url);
bool removeFirstDomain(QString& hostname);
bool handleURL(const QString& entryUrl, const QString& hostname, const QString& url);
bool handleURL(const QString& entryUrl, const QString& url, const QString& submitUrl);
QString baseDomain(const QString& hostname) const;
QSharedPointer<Database> getDatabase();
QSharedPointer<Database> selectedDatabase();

273
tests/TestBrowser.cpp
View File

@ -179,29 +179,23 @@ void TestBrowser::testSearchEntries()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
QList<QString> urls;
urls.push_back("https://github.com/login_page");
urls.push_back("https://github.com/login");
urls.push_back("https://github.com/");
urls.push_back("github.com/login");
urls.push_back("http://github.com");
urls.push_back("http://github.com/login");
urls.push_back("github.com");
urls.push_back("github.com/login");
urls.push_back("https://github"); // Invalid URL
urls.push_back("github.com");
QStringList urls = {
"https://github.com/login_page",
"https://github.com/login",
"https://github.com/",
"github.com/login",
"http://github.com",
"http://github.com/login",
"github.com",
"github.com/login",
"https://github", // Invalid URL
"github.com"
};
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
}
createEntries(urls, root);
browserSettings()->setMatchUrlScheme(false);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session"); // db, url, submitUrl
QCOMPARE(result.length(), 9);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
@ -213,7 +207,7 @@ void TestBrowser::testSearchEntries()
// With matching there should be only 3 results + 4 without a scheme
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 7);
QCOMPARE(result[0]->url(), QString("https://github.com/login_page"));
QCOMPARE(result[1]->url(), QString("https://github.com/login"));
@ -226,22 +220,16 @@ void TestBrowser::testSearchEntriesWithPort()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
QList<QString> urls;
urls.push_back("http://127.0.0.1:443");
urls.push_back("http://127.0.0.1:80");
QStringList urls = {
"http://127.0.0.1:443",
"http://127.0.0.1:80"
};
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
}
createEntries(urls, root);
auto result = m_browserService->searchEntries(db, "127.0.0.1", "http://127.0.0.1:443"); // db, hostname, url
auto result = m_browserService->searchEntries(db, "http://127.0.0.1:443", "http://127.0.0.1");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[0]->url(), QString("http://127.0.0.1:443"));
}
void TestBrowser::testSearchEntriesWithAdditionalURLs()
@ -249,70 +237,59 @@ void TestBrowser::testSearchEntriesWithAdditionalURLs()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
QList<Entry*> entries;
QList<QString> urls;
urls.push_back("https://github.com/");
urls.push_back("https://www.example.com");
urls.push_back("http://domain.com");
QStringList urls = {
"https://github.com/",
"https://www.example.com",
"http://domain.com"
};
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
entries.push_back(entry);
}
auto entries = createEntries(urls, root);
// Add an additional URL to the first entry
entries.first()->attributes()->set(BrowserService::ADDITIONAL_URL, "https://keepassxc.org");
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[0]->url(), QString("https://github.com/"));
// Search the additional URL. It should return the same entry
auto additionalResult = m_browserService->searchEntries(db, "keepassxc.org", "https://keepassxc.org");
auto additionalResult = m_browserService->searchEntries(db, "https://keepassxc.org", "https://keepassxc.org");
QCOMPARE(additionalResult.length(), 1);
QCOMPARE(additionalResult[0]->url(), urls[0]);
QCOMPARE(additionalResult[0]->url(), QString("https://github.com/"));
}
void TestBrowser::testInvalidEntries()
{
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
const QString url("https://github.com");
const QString submitUrl("https://github.com/session");
QList<QString> urls;
urls.push_back("https://github.com/login");
urls.push_back("https:///github.com/"); // Extra '/'
urls.push_back("http://github.com/**//*");
urls.push_back("http://*.github.com/login");
urls.push_back("//github.com"); // fromUserInput() corrects this one.
urls.push_back("github.com/{}<>");
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
}
QStringList urls = {
"https://github.com/login",
"https:///github.com/", // Extra '/'
"http://github.com/**//*",
"http://*.github.com/login",
"//github.com", // fromUserInput() corrects this one.
"github.com/{}<>",
"http:/example.com",
};
createEntries(urls, root);
browserSettings()->setMatchUrlScheme(true);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 2);
QCOMPARE(result[0]->url(), QString("https://github.com/login"));
QCOMPARE(result[1]->url(), QString("//github.com"));
// Test the URL's directly
QCOMPARE(m_browserService->handleURL(urls[0], "github.com", "https://github.com"), true);
QCOMPARE(m_browserService->handleURL(urls[1], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[2], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[3], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[4], "github.com", "https://github.com"), true);
QCOMPARE(m_browserService->handleURL(urls[5], "github.com", "https://github.com"), false);
QCOMPARE(m_browserService->handleURL(urls[0], url, submitUrl), true);
QCOMPARE(m_browserService->handleURL(urls[1], url, submitUrl), false);
QCOMPARE(m_browserService->handleURL(urls[2], url, submitUrl), false);
QCOMPARE(m_browserService->handleURL(urls[3], url, submitUrl), false);
QCOMPARE(m_browserService->handleURL(urls[4], url, submitUrl), true);
QCOMPARE(m_browserService->handleURL(urls[5], url, submitUrl), false);
}
void TestBrowser::testSubdomainsAndPaths()
@ -320,44 +297,74 @@ void TestBrowser::testSubdomainsAndPaths()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
QList<QString> urls;
urls.push_back("https://www.github.com/login/page.xml");
urls.push_back("https://login.github.com/");
urls.push_back("https://github.com");
urls.push_back("http://www.github.com");
urls.push_back("http://login.github.com/pathtonowhere");
urls.push_back(".github.com"); // Invalid URL
urls.push_back("www.github.com/");
urls.push_back("https://github"); // Invalid URL
QStringList urls = {
"https://www.github.com/login/page.xml",
"https://login.github.com/",
"https://github.com",
"http://www.github.com",
"http://login.github.com/pathtonowhere",
".github.com", // Invalid URL
"www.github.com/",
"https://github" // Invalid URL
};
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
}
createEntries(urls, root);
browserSettings()->setMatchUrlScheme(false);
auto result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
auto result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("https://github.com"));
QCOMPARE(result.length(), 6);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[1]->url(), urls[1]);
QCOMPARE(result[2]->url(), urls[2]);
QCOMPARE(result[3]->url(), urls[3]);
QCOMPARE(result[4]->url(), urls[4]);
QCOMPARE(result[5]->url(), urls[6]);
// With matching there should be only 3 results
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "github.com", "https://github.com"); // db, hostname, url
// With www subdomain
result = m_browserService->searchEntries(db, "https://www.github.com", "https://www.github.com/session");
QCOMPARE(result.length(), 4);
QCOMPARE(result[0]->url(), urls[0]);
QCOMPARE(result[1]->url(), urls[1]);
QCOMPARE(result[2]->url(), urls[2]);
QCOMPARE(result[3]->url(), urls[6]);
QCOMPARE(result[0]->url(), QString("https://www.github.com/login/page.xml"));
QCOMPARE(result[1]->url(), QString("https://github.com")); // Accepts any subdomain
QCOMPARE(result[2]->url(), QString("http://www.github.com"));
QCOMPARE(result[3]->url(), QString("www.github.com/"));
// With scheme matching there should be only 1 result
browserSettings()->setMatchUrlScheme(true);
result = m_browserService->searchEntries(db, "https://github.com", "https://github.com/session");
QCOMPARE(result.length(), 1);
QCOMPARE(result[0]->url(), QString("https://github.com"));
// Test site with subdomain in the site URL
QStringList entryURLs = {
"https://accounts.example.com",
"https://accounts.example.com/path",
"https://subdomain.example.com/",
"https://another.accounts.example.com/",
"https://another.subdomain.example.com/",
"https://example.com/",
"https://example" // Invalid URL
};
createEntries(entryURLs, root);
result = m_browserService->searchEntries(db, "https://accounts.example.com", "https://accounts.example.com");
QCOMPARE(result.length(), 3);
QCOMPARE(result[0]->url(), QString("https://accounts.example.com"));
QCOMPARE(result[1]->url(), QString("https://accounts.example.com/path"));
QCOMPARE(result[2]->url(), QString("https://example.com/")); // Accepts any subdomain
result = m_browserService->searchEntries(db, "https://another.accounts.example.com", "https://another.accounts.example.com");
QCOMPARE(result.length(), 4);
QCOMPARE(result[0]->url(), QString("https://accounts.example.com")); // Accepts any subdomain under accounts.example.com
QCOMPARE(result[1]->url(), QString("https://accounts.example.com/path"));
QCOMPARE(result[2]->url(), QString("https://another.accounts.example.com/"));
QCOMPARE(result[3]->url(), QString("https://example.com/")); // Accepts one or more subdomains
// Test local files. It should be a direct match.
QStringList localFiles = {
"file:///Users/testUser/tests/test.html"
};
createEntries(localFiles, root);
// With local files, url is always set to the file scheme + ://. Submit URL holds the actual URL.
result = m_browserService->searchEntries(db, "file://", "file:///Users/testUser/tests/test.html");
QCOMPARE(result.length(), 1);
}
void TestBrowser::testSortEntries()
@ -365,28 +372,20 @@ void TestBrowser::testSortEntries()
auto db = QSharedPointer<Database>::create();
auto* root = db->rootGroup();
QList<QString> urls;
urls.push_back("https://github.com/login_page");
urls.push_back("https://github.com/login");
urls.push_back("https://github.com/");
urls.push_back("github.com/login");
urls.push_back("http://github.com");
urls.push_back("http://github.com/login");
urls.push_back("github.com");
urls.push_back("github.com/login");
urls.push_back("https://github"); // Invalid URL
urls.push_back("github.com");
QStringList urls = {
"https://github.com/login_page",
"https://github.com/login",
"https://github.com/",
"github.com/login",
"http://github.com",
"http://github.com/login",
"github.com",
"github.com/login",
"https://github", // Invalid URL
"github.com"
};
QList<Entry*> entries;
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
entries.push_back(entry);
}
auto entries = createEntries(urls, root);
browserSettings()->setBestMatchOnly(false);
auto result =
@ -458,3 +457,19 @@ void TestBrowser::testGetDatabaseGroups()
auto lastChild = lastChildren.at(0);
QCOMPARE(lastChild.toObject()["name"].toString(), QString("group2_1_1"));
}
QList<Entry*> TestBrowser::createEntries(QStringList& urls, Group* root) const
{
QList<Entry*> entries;
for (int i = 0; i < urls.length(); ++i) {
auto entry = new Entry();
entry->setGroup(root);
entry->beginUpdate();
entry->setUrl(urls[i]);
entry->setUsername(QString("User %1").arg(i));
entry->endUpdate();
entries.push_back(entry);
}
return entries;
}

2
tests/TestBrowser.h
View File

@ -49,6 +49,8 @@ private slots:
void testGetDatabaseGroups();
private:
QList<Entry*> createEntries(QStringList& urls, Group* root) const;
QScopedPointer<BrowserAction> m_browserAction;
QScopedPointer<BrowserService> m_browserService;
};