diff options
author | Christian Mollekopf <chrigi_1@fastmail.fm> | 2017-05-03 21:29:28 +0200 |
---|---|---|
committer | Christian Mollekopf <chrigi_1@fastmail.fm> | 2017-05-03 21:29:28 +0200 |
commit | 9228b3ba170a0f68dbb432b2455c75d5fff21506 (patch) | |
tree | 0bfc3231b050efcfb7c5aec0664bd4739a6266f2 | |
parent | ca5020095abfb76e63bd801e9722c07193eb05f5 (diff) | |
download | sink-9228b3ba170a0f68dbb432b2455c75d5fff21506.tar.gz sink-9228b3ba170a0f68dbb432b2455c75d5fff21506.zip |
Sanity check db names
lmdb and sink deal badly with e.g. a string containing a null in the
millde as db name. Thus we try to protect better against it.
This is an actual problem we triggered: https://phabricator.kde.org/T5880
-rw-r--r-- | common/storage/entitystore.cpp | 5 | ||||
-rw-r--r-- | common/storage_lmdb.cpp | 20 |
2 files changed, 25 insertions, 0 deletions
diff --git a/common/storage/entitystore.cpp b/common/storage/entitystore.cpp index b7309ab..4cb4641 100644 --- a/common/storage/entitystore.cpp +++ b/common/storage/entitystore.cpp | |||
@@ -320,6 +320,11 @@ void EntityStore::cleanupEntityRevisionsUntil(qint64 revision) | |||
320 | { | 320 | { |
321 | const auto uid = DataStore::getUidFromRevision(d->transaction, revision); | 321 | const auto uid = DataStore::getUidFromRevision(d->transaction, revision); |
322 | const auto bufferType = DataStore::getTypeFromRevision(d->transaction, revision); | 322 | const auto bufferType = DataStore::getTypeFromRevision(d->transaction, revision); |
323 | if (bufferType.isEmpty() || uid.isEmpty()) { | ||
324 | SinkErrorCtx(d->logCtx) << "Failed to find revision during cleanup: " << revision; | ||
325 | Q_ASSERT(false); | ||
326 | return; | ||
327 | } | ||
323 | SinkTraceCtx(d->logCtx) << "Cleaning up revision " << revision << uid << bufferType; | 328 | SinkTraceCtx(d->logCtx) << "Cleaning up revision " << revision << uid << bufferType; |
324 | DataStore::mainDatabase(d->transaction, bufferType) | 329 | DataStore::mainDatabase(d->transaction, bufferType) |
325 | .scan(uid, | 330 | .scan(uid, |
diff --git a/common/storage_lmdb.cpp b/common/storage_lmdb.cpp index 08eea37..18364ea 100644 --- a/common/storage_lmdb.cpp +++ b/common/storage_lmdb.cpp | |||
@@ -169,6 +169,26 @@ public: | |||
169 | if (const int rc = mdb_dbi_open(transaction, db.constData(), flags, &dbi)) { | 169 | if (const int rc = mdb_dbi_open(transaction, db.constData(), flags, &dbi)) { |
170 | //Create the db if it is not existing already | 170 | //Create the db if it is not existing already |
171 | if (rc == MDB_NOTFOUND && !readOnly) { | 171 | if (rc == MDB_NOTFOUND && !readOnly) { |
172 | //Sanity check db name | ||
173 | { | ||
174 | auto parts = db.split('.'); | ||
175 | for (const auto &p : parts) { | ||
176 | auto containsSpecialCharacter = [] (const QByteArray &p) { | ||
177 | for (int i = 0; i < p.size(); i++) { | ||
178 | const auto c = p.at(i); | ||
179 | //Between 0 and z in the ascii table. Essentially ensures that the name is printable and doesn't contain special chars | ||
180 | if (c < 0x30 || c > 0x7A) { | ||
181 | return true; | ||
182 | } | ||
183 | } | ||
184 | return false; | ||
185 | }; | ||
186 | if (p.isEmpty() || containsSpecialCharacter(p)) { | ||
187 | SinkError() << "Tried to create a db with an invalid name. Hex:" << db.toHex() << " ASCII:" << db; | ||
188 | Q_ASSERT(false); | ||
189 | } | ||
190 | } | ||
191 | } | ||
172 | if (const int rc = mdb_dbi_open(transaction, db.constData(), flags | MDB_CREATE, &dbi)) { | 192 | if (const int rc = mdb_dbi_open(transaction, db.constData(), flags | MDB_CREATE, &dbi)) { |
173 | SinkWarning() << "Failed to create db " << QByteArray(mdb_strerror(rc)); | 193 | SinkWarning() << "Failed to create db " << QByteArray(mdb_strerror(rc)); |
174 | Error error(name.toLatin1(), ErrorCodes::GenericError, "Error while creating database: " + QByteArray(mdb_strerror(rc))); | 194 | Error error(name.toLatin1(), ErrorCodes::GenericError, "Error while creating database: " + QByteArray(mdb_strerror(rc))); |