[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kactivities] /: Check validity of arguments so that we don't get SQL injection
From:       Ivan Čukić <ivan.cukic () kde ! org>
Date:       2015-05-31 22:28:26
Message-ID: E1YzBiU-0002o5-5N () scm ! kde ! org
[Download RAW message or body]

Git commit 479540375e54996aaca8311a8fbbd2aba72d8ab7 by Ivan Čukić.
Committed on 31/05/2015 at 21:23.
Pushed by ivan into branch 'master'.

Check validity of arguments so that we don't get SQL injection

M  +10   -20   TODO
M  +0    -4    src/lib/stats/resultset.cpp
M  +2    -1    src/service/plugins/sqlite/StatsPlugin.cpp

http://commits.kde.org/kactivities/479540375e54996aaca8311a8fbbd2aba72d8ab7

diff --git a/TODO b/TODO
index 91e85e7..ff71993 100644
--- a/TODO
+++ b/TODO
@@ -15,33 +15,25 @@ src/lib/core/resourceinstance.cpp:159:
 
 libKActivitiesStats
 ===================
-src/lib/stats/resultset.cpp:171:
- TODO: Implement types
- QStringList types() const;
-src/lib/stats/resultset.cpp:232:
+src/lib/stats/resultset.cpp:245:
  TODO: We need to correct the scores based on the time that passed
        since the cache was last updated, although, for this query,
        scores are not that important.
-src/lib/stats/resultset.cpp:307:
+src/lib/stats/resultset.cpp:321:
  TODO: We need to correct the scores based on the time that passed
        since the cache was last updated
-src/lib/stats/resultset.cpp:376:
+src/lib/stats/resultset.cpp:392:
  TODO: Get this to work
 
 src/lib/stats/resultwatcher.h:91:
  TODO: Move these to libKActivities
 
-src/lib/stats/resultmodel.cpp:144:
+src/lib/stats/resultmodel.cpp:407:
  TODO: We should also sort by the resource, not only on a single field
-src/lib/stats/resultmodel.cpp:237:
+src/lib/stats/resultmodel.cpp:493:
  TODO: This can add or remove items from the model
-src/lib/stats/resultmodel.cpp:253:
- TODO: Make this a little bit smarter
-       - there is a possibility that the new list of
-       items will not differ significantly to the old one
-       in which case it does not need to be a full model reset
 
-src/lib/stats/resultwatcher.cpp:170:
+src/lib/stats/resultwatcher.cpp:174:
  TODO: See whether it makes sense to have lastUpdate/firstUpdate here as well
 
 
@@ -59,8 +51,6 @@ src/service/plugins/sqlite/StatsPlugin.cpp:117:
        not only on startup
 src/service/plugins/sqlite/StatsPlugin.cpp:408:
  TODO: Add focus and modification
-src/service/plugins/sqlite/StatsPlugin.cpp:545:
- TODO: Check against sql injection
 
 src/service/Application.cpp:155:
  TODO: Restart on crash
@@ -71,18 +61,18 @@ src/service/Application.cpp:197:
 
 QML imports
 ===========
-src/imports/resourcemodel.cpp:104:
+src/imports/resourcemodel.cpp:105:
  NOTE: What to do if the file does not exist?
        Ignoring that case since the daemon creates it on startup.
        Is it plausible that somebody will instantiate the ResourceModel
        before the daemon is started?
-src/imports/resourcemodel.cpp:134:
+src/imports/resourcemodel.cpp:135:
  TODO: Database connection naming could be smarter (thread-id-based,
        reusing connections...?)
-src/imports/resourcemodel.cpp:336:
+src/imports/resourcemodel.cpp:337:
  TODO: Will probably need some more special handling -
        for application:/ and a few more
-src/imports/resourcemodel.cpp:552:
+src/imports/resourcemodel.cpp:553:
  TODO: This might be smarter possibly, but might collide
        with the SQL model. Implement a custom model with internal
        cache instead of basing it on QSqlModel.
diff --git a/src/lib/stats/resultset.cpp b/src/lib/stats/resultset.cpp
index 2f0d369..236288d 100644
--- a/src/lib/stats/resultset.cpp
+++ b/src/lib/stats/resultset.cpp
@@ -167,10 +167,6 @@ public:
               : QString());
 
         Q_ASSERT_X(query.isActive(), "ResultSet initQuery", "Query is not valid");
-
-        // TODO: Implement types
-        // QStringList types() const;
-
     }
 
     QString agentClause(const QString &agent) const
diff --git a/src/service/plugins/sqlite/StatsPlugin.cpp b/src/service/plugins/sqlite/StatsPlugin.cpp
index ef13907..6cd5dad 100644
--- a/src/service/plugins/sqlite/StatsPlugin.cpp
+++ b/src/service/plugins/sqlite/StatsPlugin.cpp
@@ -542,7 +542,8 @@ void StatsPlugin::DeleteStatsForResource(const QString &activity,
 
     DATABASE_TRANSACTION(resourcesDatabase());
 
-    // TODO: Check against sql injection
+    // Check against sql injection
+    if (activity.contains('\'') || client.contains('\'')) return;
 
     const auto activityFilter =
             activity == ANY_ACTIVITY_TAG ? " 1 " :

[prev in list] [next in list] [prev in thread] [next in thread]