Skip to content

Commit 7b4749a

Browse files
committed
Bug: 467378 - Char escape in DatabaseMessageStore
The existing SQLite statements were constructed in such a way that the client ID could inject SQL into the statement. I have modified them to be proper prepared statements thus allowing safe usage of ' in client IDs. Signed-off-by: James Sutton <james.sutton@uk.ibm.com>
1 parent bf423fb commit 7b4749a

1 file changed

Lines changed: 50 additions & 17 deletions

File tree

  • org.eclipse.paho.android.service/org.eclipse.paho.android.service/src/main/java/org/eclipse/paho/android/service

org.eclipse.paho.android.service/org.eclipse.paho.android.service/src/main/java/org/eclipse/paho/android/service/DatabaseMessageStore.java

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
* http://www.eclipse.org/legal/epl-v10.html
1010
* and the Eclipse Distribution License is available at
1111
* http://www.eclipse.org/org/documents/edl-v10.php.
12+
*
13+
* Contributors:
14+
* James Sutton - Removing SQL Injection vunerability (bug 467378)
1215
*/
1316
package org.eclipse.paho.android.service;
1417

@@ -203,12 +206,23 @@ public String storeArrived(String clientHandle, String topic,
203206
}
204207

205208
private int getArrivedRowCount(String clientHandle) {
206-
String[] cols = new String[1];
207-
cols[0] = "COUNT(*)";
208-
Cursor c = db.query(ARRIVED_MESSAGE_TABLE_NAME, cols,
209-
MqttServiceConstants.CLIENT_HANDLE + "='" + clientHandle + "'",
210-
null, null, null, null);
211-
int count = 0;
209+
int count = 0;
210+
String[] projection = {
211+
MqttServiceConstants.MESSAGE_ID,
212+
};
213+
String selection = MqttServiceConstants.CLIENT_HANDLE;
214+
String[] selectionArgs = new String[1];
215+
selectionArgs[0] = clientHandle;
216+
Cursor c = db.query(
217+
ARRIVED_MESSAGE_TABLE_NAME, // Table Name
218+
projection, // The columns to return;
219+
selection, // Columns for WHERE Clause
220+
selectionArgs , // The values for the WHERE Cause
221+
null, //Don't group the rows
222+
null, // Don't filter by row groups
223+
null // The sort order
224+
);
225+
212226
if (c.moveToFirst()) {
213227
count = c.getInt(0);
214228
}
@@ -234,11 +248,15 @@ public boolean discardArrived(String clientHandle, String id) {
234248
traceHandler.traceDebug(TAG, "discardArrived{" + clientHandle + "}, {"
235249
+ id + "}");
236250
int rows;
251+
String[] selectionArgs = new String[2];
252+
selectionArgs[0] = id;
253+
selectionArgs[1] = clientHandle;
254+
237255
try {
238256
rows = db.delete(ARRIVED_MESSAGE_TABLE_NAME,
239-
MqttServiceConstants.MESSAGE_ID + "='" + id + "' AND "
240-
+ MqttServiceConstants.CLIENT_HANDLE + "='"
241-
+ clientHandle + "'", null);
257+
MqttServiceConstants.MESSAGE_ID + "=? AND "
258+
+ MqttServiceConstants.CLIENT_HANDLE + "=?",
259+
selectionArgs);
242260
} catch (SQLException e) {
243261
traceHandler.traceException(TAG, "discardArrived", e);
244262
throw e;
@@ -272,18 +290,30 @@ public Iterator<StoredMessage> getAllArrivedMessages(
272290
return new Iterator<StoredMessage>() {
273291
private Cursor c;
274292
private boolean hasNext;
293+
private String[] selectionArgs = {
294+
clientHandle,
295+
};
296+
275297

276298
{
277299
db = mqttDb.getWritableDatabase();
278300
// anonymous initialiser to start a suitable query
279301
// and position at the first row, if one exists
280302
if (clientHandle == null) {
281-
c = db.query(ARRIVED_MESSAGE_TABLE_NAME, null, null, null,
282-
null, null, "mtimestamp ASC");
303+
c = db.query(ARRIVED_MESSAGE_TABLE_NAME,
304+
null,
305+
null,
306+
null,
307+
null,
308+
null,
309+
"mtimestamp ASC");
283310
} else {
284-
c = db.query(ARRIVED_MESSAGE_TABLE_NAME, null,
285-
MqttServiceConstants.CLIENT_HANDLE + "='"
286-
+ clientHandle + "'", null, null, null,
311+
c = db.query(ARRIVED_MESSAGE_TABLE_NAME,
312+
null,
313+
MqttServiceConstants.CLIENT_HANDLE + "=?",
314+
selectionArgs,
315+
null,
316+
null,
287317
"mtimestamp ASC");
288318
}
289319
hasNext = c.moveToFirst();
@@ -352,6 +382,8 @@ protected void finalize() throws Throwable {
352382
public void clearArrivedMessages(String clientHandle) {
353383

354384
db = mqttDb.getWritableDatabase();
385+
String[] selectionArgs = new String[1];
386+
selectionArgs[0] = clientHandle;
355387

356388
int rows = 0;
357389
if (clientHandle == null) {
@@ -362,9 +394,10 @@ public void clearArrivedMessages(String clientHandle) {
362394
traceHandler.traceDebug(TAG,
363395
"clearArrivedMessages: clearing the table of "
364396
+ clientHandle + " messages");
365-
rows = db.delete(ARRIVED_MESSAGE_TABLE_NAME,
366-
MqttServiceConstants.CLIENT_HANDLE + "='" + clientHandle
367-
+ "'", null);
397+
rows = db.delete(ARRIVED_MESSAGE_TABLE_NAME,
398+
MqttServiceConstants.CLIENT_HANDLE + "=?",
399+
selectionArgs);
400+
368401
}
369402
traceHandler.traceDebug(TAG, "clearArrivedMessages: rows affected = "
370403
+ rows);

0 commit comments

Comments
 (0)