Fixed sql injection bug using sqlite prepared statements.

added more doc detail to contentvalue (put takes private copy of data).
binds to content pointers take private copy of content.

git-svn-id: http://svn.code.sf.net/p/retroshare/code/trunk@6320 b45a01b8-16f6-495d-af2f-9b41ad6348cc

libretroshare/src/libretroshare.pro

@@ -632,6 +632,7 @@ gxs {
 		retroshare/rsgxsservice.h \
 		serialiser/rsgxsitems.h \
 		util/retrodb.h \
+		util/rsdbbind.h \
 		gxs/rsgxsutil.h \
 		util/contentvalue.h \
 		gxs/gxssecurity.h \
@@ -649,10 +650,12 @@ gxs {
 		gxs/rsgxsdataaccess.cc \
 		util/retrodb.cc \
 		util/contentvalue.cc \
+		util/rsdbbind.cc \
 		gxs/gxssecurity.cc \
 		gxs/gxstokenqueue.cc \
 		gxs/rsgxsutil.cc
 
+
 	# Identity Service
 	HEADERS += retroshare/rsidentity.h \
 		gxs/rsgixs.h \

libretroshare/src/serialiser/rsgxsitems.cc

@@ -23,7 +23,7 @@
 		this->mParentId = rGxsMeta.mParentId;
 		this->mPublishTs = rGxsMeta.mPublishTs;
 		this->mThreadId = rGxsMeta.mThreadId;
-                this->mServiceString = rGxsMeta.mServiceString;
+		this->mServiceString = rGxsMeta.mServiceString;
 
 	}
 
@@ -40,12 +40,12 @@
 		this->mPublishTs = rGxsMeta.mPublishTs;
 		this->mSubscribeFlags = rGxsMeta.mSubscribeFlags;
 		this->mGroupName = rGxsMeta.mGroupName;
-                this->mServiceString = rGxsMeta.mServiceString;
+		this->mServiceString = rGxsMeta.mServiceString;
-                this->mSignFlags = rGxsMeta.mSignFlags;
+		this->mSignFlags = rGxsMeta.mSignFlags;
-                this->mCircleId = rGxsMeta.mCircleId;
+		this->mCircleId = rGxsMeta.mCircleId;
-                this->mCircleType = rGxsMeta.mCircleType;
+		this->mCircleType = rGxsMeta.mCircleType;
-                this->mInternalCircle = rGxsMeta.mInternalCircle;
+		this->mInternalCircle = rGxsMeta.mInternalCircle;
-                this->mOriginator = rGxsMeta.mOriginator;
+		this->mOriginator = rGxsMeta.mOriginator;
 	}
 
 

libretroshare/src/util/contentvalue.h

@@ -99,6 +99,7 @@ public:
 
     /*!
      * Adds a value to the set
+     * Takes a private copy of data
      * @param key  the name of the value to put
      * @param value  the data for the value to put
      */

libretroshare/src/util/retrodb.cc

@@ -2,7 +2,7 @@
 /*
  * RetroShare : RetroDb functionality
  *
- * Copyright 2012 Christopher Evi-Parker
+ * Copyright 2012-2013 Christopher Evi-Parker
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -30,19 +30,11 @@
 
 #include "retrodb.h"
 #include "radix64.h"
+#include "rsdbbind.h"
 
 //#define RETRODB_DEBUG
-//#define RADIX_STRING
 
 
-void free_blob(void* dat){
-
-    char* c = (char*) dat;
-    delete[] c;
-    dat = NULL;
-
-}
-
 const int RetroDb::OPEN_READONLY = SQLITE_OPEN_READONLY;
 const int RetroDb::OPEN_READWRITE = SQLITE_OPEN_READWRITE;
 const int RetroDb::OPEN_READWRITE_CREATE = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
@@ -195,7 +187,7 @@ bool RetroDb::isOpen() const {
     return (mDb==NULL ? false : true);
 }
 
-bool RetroDb::sqlInsert(const std::string &table, const std::string& nullColumnHack, const ContentValue &cv){
+bool RetroDb::sqlInsert(const std::string &table, const std::string& /* nullColumnHack */, const ContentValue &cv){
 
     std::map<std::string, uint8_t> keyTypeMap;
     cv.getKeyTypeMap(keyTypeMap);
@@ -214,102 +206,30 @@ bool RetroDb::sqlInsert(const std::string &table, const std::string& nullColumnH
         if(mit == keyTypeMap.end())
             qColumns += ")"; // close bracket if at end
         else
-             qColumns += ",";
+            qColumns += ",";
 
         mit--;
     }
 
     // build values part of insertion
-    std::string qValues = "VALUES(";
+    std::string qValues;
-    std::ostringstream oStrStream;
+    std::list<RetroBind*> paramBindings;
-    uint32_t index = 0;
+    buildInsertQueryValue(keyTypeMap, cv, qValues, paramBindings);
-    std::list<RetroDbBlob> blobL;
-
-    for(mit=keyTypeMap.begin(); mit!=keyTypeMap.end(); mit++){
-
-        uint8_t type = mit->second;
-        std::string key = mit->first;
-
-
-
-        if(ContentValue::BOOL_TYPE == type)
-            {
-                bool value;
-                cv.getAsBool(key, value);
-                oStrStream << value;
-                qValues += oStrStream.str();
-            }
-        else if( ContentValue::DOUBLE_TYPE == type)
-            {
-                double value;
-                cv.getAsDouble(key, value);
-                oStrStream << value;
-                qValues += oStrStream.str();
-            }
-        else if( ContentValue::DATA_TYPE == type)
-            {
-                char* value;
-                uint32_t len;
-                cv.getAsData(key, len, value);
-                RetroDbBlob b;
-                b.data = value;
-                b.length = len;
-                b.index = ++index;
-                blobL.push_back(b);
-                qValues += "?"; // parameter
-            }
-        else if ( ContentValue::STRING_TYPE == type)
-            {
-                std::string value;
-                cv.getAsString(key, value);
-#ifdef RADIX_STRING
-                Radix64::encode(value.c_str(), value.size(), value);
-#endif
-                qValues += "'" + value +"'";
-            }
-        else if ( ContentValue::INT32_TYPE == type)
-            {
-                int32_t value = 0;
-                cv.getAsInt32(key, value);
-                oStrStream << value;
-                qValues += oStrStream.str();
-            }
-        else if( ContentValue::INT64_TYPE == type)
-            {
-                int64_t value = 0;
-                cv.getAsInt64(key, value);
-                oStrStream << value;
-                qValues += oStrStream.str();
-            }
-
-
-        mit++;
-        if(mit != keyTypeMap.end()){ // add comma if more columns left
-            qValues += ",";
-        }
-        else{ // at end close brackets
-             qValues += ");";
-         }
-        mit--;
-
-        // clear stream strings
-        oStrStream.str("");
-    }
-
 
     // complete insertion query
     std::string sqlQuery = "INSERT INTO " + qColumns + " " + qValues;
 
+    execSQL_bind(sqlQuery, paramBindings);
+
 #ifdef RETRODB_DEBUG
     std::cerr << "RetroDb::sqlInsert(): " << sqlQuery << std::endl;
 #endif
 
-    // execute query
+
-    execSQL_bind_blobs(sqlQuery, blobL);
     return true;
 }
 
-bool RetroDb::execSQL_bind_blobs(const std::string &query, std::list<RetroDbBlob> &blobs){
+bool RetroDb::execSQL_bind(const std::string &query, std::list<RetroBind*> &paramBindings){
 
     // prepare statement
     sqlite3_stmt* stm = NULL;
@@ -328,11 +248,19 @@ bool RetroDb::execSQL_bind_blobs(const std::string &query, std::list<RetroDbBlob
         return false;
     }
 
-    std::list<RetroDbBlob>::iterator lit = blobs.begin();
+    std::list<RetroBind*>::iterator lit = paramBindings.begin();
 
-    for(; lit != blobs.end(); lit++){
+    for(; lit != paramBindings.end(); lit++){
-        const RetroDbBlob& b = *lit;
+        RetroBind* rb = *lit;
-        sqlite3_bind_blob(stm, b.index, b.data, b.length, NULL);
+
+        if(!rb->bind(stm))
+        {
+        	std::cerr << "\nBind failed for index: " << rb->getIndex()
+        			  << std::endl;
+        }
+
+        delete rb;
+        rb = NULL;
     }
 
     uint32_t delta = 3;
@@ -381,7 +309,149 @@ bool RetroDb::execSQL_bind_blobs(const std::string &query, std::list<RetroDbBlob
     return ok;
 }
 
-bool RetroDb::sqlDelete(const std::string &tableName, const std::string &whereClause, const std::string &whereArgs){
+void RetroDb::buildInsertQueryValue(const std::map<std::string, uint8_t> keyTypeMap,
+		const ContentValue& cv, std::string& parameter,
+		std::list<RetroBind*>& paramBindings)
+{
+	std::map<std::string, uint8_t>::const_iterator mit = keyTypeMap.begin();
+
+	parameter = "VALUES(";
+	int index = 0;
+    for(mit=keyTypeMap.begin(); mit!=keyTypeMap.end(); mit++)
+    {
+
+        uint8_t type = mit->second;
+        std::string key = mit->first;
+
+        RetroBind* rb = NULL;
+        if(ContentValue::BOOL_TYPE == type)
+            {
+                bool value;
+                cv.getAsBool(key, value);
+                rb = new RsBoolBind(value, ++index);
+            }
+        else if( ContentValue::DOUBLE_TYPE == type)
+            {
+                double value;
+                cv.getAsDouble(key, value);
+                rb = new RsDoubleBind(value, ++index);
+            }
+        else if( ContentValue::DATA_TYPE == type)
+            {
+                char* value;
+                uint32_t len;
+                cv.getAsData(key, len, value);
+                rb = new RsBlobBind(value, len, ++index);
+            }
+        else if ( ContentValue::STRING_TYPE == type)
+            {
+                std::string value;
+                cv.getAsString(key, value);
+                rb = new RsStringBind(value, ++index);
+            }
+        else if ( ContentValue::INT32_TYPE == type)
+            {
+                int32_t value = 0;
+                cv.getAsInt32(key, value);
+                rb = new RsInt32Bind(value, ++index);
+            }
+        else if( ContentValue::INT64_TYPE == type)
+            {
+                int64_t value = 0;
+                cv.getAsInt64(key, value);
+                rb = new RsInt64bind(value, ++index);
+            }
+
+        if(rb)
+        {
+        	paramBindings.push_back(rb);
+
+        	mit++;
+
+        	if(mit == keyTypeMap.end())
+        		parameter += "?";
+        	else
+        		parameter += "?,";
+
+        	mit--;
+        }
+    }
+
+    parameter += ")";
+
+
+}
+
+void RetroDb::buildUpdateQueryValue(const std::map<std::string, uint8_t> keyTypeMap,
+		const ContentValue& cv, std::string& parameter,
+		std::list<RetroBind*>& paramBindings)
+{
+	std::map<std::string, uint8_t>::const_iterator mit = keyTypeMap.begin();
+
+	int index = 0;
+    for(mit=keyTypeMap.begin(); mit!=keyTypeMap.end(); mit++)
+    {
+
+        uint8_t type = mit->second;
+        std::string key = mit->first;
+
+        RetroBind* rb = NULL;
+        if(ContentValue::BOOL_TYPE == type)
+            {
+                bool value;
+                cv.getAsBool(key, value);
+                rb = new RsBoolBind(value, ++index);
+            }
+        else if( ContentValue::DOUBLE_TYPE == type)
+            {
+                double value;
+                cv.getAsDouble(key, value);
+                rb = new RsDoubleBind(value, ++index);
+            }
+        else if( ContentValue::DATA_TYPE == type)
+            {
+                char* value;
+                uint32_t len;
+                cv.getAsData(key, len, value);
+                rb = new RsBlobBind(value, len, ++index);
+            }
+        else if ( ContentValue::STRING_TYPE == type)
+            {
+                std::string value;
+                cv.getAsString(key, value);
+                rb = new RsStringBind(value, ++index);
+            }
+        else if ( ContentValue::INT32_TYPE == type)
+            {
+                int32_t value = 0;
+                cv.getAsInt32(key, value);
+                rb = new RsInt32Bind(value, ++index);
+            }
+        else if( ContentValue::INT64_TYPE == type)
+            {
+                int64_t value = 0;
+                cv.getAsInt64(key, value);
+                rb = new RsInt64bind(value, ++index);
+            }
+
+        if(rb)
+        {
+        	paramBindings.push_back(rb);
+
+        	mit++;
+
+        	if(mit == keyTypeMap.end())
+        		parameter += key + "=?";
+        	else
+        		parameter += key + "=?,";
+
+        	mit--;
+        }
+    }
+
+}
+
+bool RetroDb::sqlDelete(const std::string &tableName, const std::string &whereClause, const std::string &/*whereArgs*/){
 
     std::string sqlQuery = "DELETE FROM " + tableName;
 
@@ -404,69 +474,9 @@ bool RetroDb::sqlUpdate(const std::string &tableName, std::string whereClause, c
     cv.getKeyTypeMap(keyTypeMap);
 
     // build SET part of update
-    std::string qValues = "";
+    std::string qValues;
-    std::ostringstream oStrStream;
+    std::list<RetroBind*> paramBindings;
-
+    buildUpdateQueryValue(keyTypeMap, cv, qValues, paramBindings);
-    for(mit=keyTypeMap.begin(); mit!=keyTypeMap.end(); mit++){
-
-        uint8_t type = mit->second;
-        std::string key = mit->first;
-
-        if( ContentValue::BOOL_TYPE == type)
-            {
-                bool value;
-                cv.getAsBool(key, value);
-                oStrStream << value;
-                qValues += key + "='" + oStrStream.str();
-            }
-        else if( ContentValue::DOUBLE_TYPE == type)
-            {
-                double value;
-                cv.getAsDouble(key, value);
-                oStrStream << value;
-                qValues += key + "='" + oStrStream.str();
-            }
-        else if( ContentValue::DATA_TYPE == type)
-            {
-                char* value;
-                uint32_t len;
-                cv.getAsData(key, len, value);
-                oStrStream.write(value, len);
-                qValues += key + "='" + oStrStream.str() + "' ";
-            }
-        else if( ContentValue::STRING_TYPE == type)
-            {
-                std::string value;
-                cv.getAsString(key, value);
-#ifdef RADIX_STRING
-                Radix64::encode(value.c_str(), value.size(), value);
-#endif
-                qValues += key + "='" + value + "' ";
-            }
-        else if( ContentValue::INT32_TYPE == type)
-            {
-                int32_t value;
-                cv.getAsInt32(key, value);
-                oStrStream << value;
-                qValues += key + "='" + oStrStream.str() + "' ";
-            }
-        else if( ContentValue::INT64_TYPE == type)
-            {
-                int64_t value;
-                cv.getAsInt64(key, value);
-                oStrStream << value;
-                qValues += key + "='" + oStrStream.str() + "' ";
-            }
-
-        mit++;
-        if(mit != keyTypeMap.end()){ // add comma if more columns left
-            qValues += ",";
-        }
-        mit--;
-
-        // clear stream strings
-        oStrStream.str("");
-    }
 
     if(qValues.empty())
         return false;
@@ -482,13 +492,10 @@ bool RetroDb::sqlUpdate(const std::string &tableName, std::string whereClause, c
     }
 
     // execute query
-    return execSQL(sqlQuery);
+    return execSQL_bind(sqlQuery, paramBindings);
 }
 
 
-
-
-
 /********************** RetroCursor ************************/
 
 RetroCursor::RetroCursor(sqlite3_stmt *stmt)

libretroshare/src/util/retrodb.h

@@ -30,9 +30,12 @@
 #include <set>
 #include <list>
 #include <map>
+#include "rsdbbind.h"
 
 #include "contentvalue.h"
 
+
+
 class RetroCursor;
 
 /*!
@@ -141,6 +144,22 @@ public:
      */
     void vacuum();
 
+
+    /*!
+     * Build the "VALUE" part of an insertiong sql query
+     * @param parameter contains place holder query
+     * @param paramBindings
+     */
+    void buildInsertQueryValue(const std::map<std::string, uint8_t> keyMap, const ContentValue& cv,
+    		std::string& parameter, std::list<RetroBind*>& paramBindings);
+
+    /*!
+     * Build the "VALUE" part of an insertiong sql query
+     * @param parameter contains place holder query
+     * @param paramBindings
+     */
+    void buildUpdateQueryValue(const std::map<std::string, uint8_t> keyMap, const ContentValue& cv,
+    		std::string& parameter, std::list<RetroBind*>& paramBindings);
 public:
 
     static const int OPEN_READONLY;
@@ -149,16 +168,7 @@ public:
 
 private:
 
-    class RetroDbBlob{
+    bool execSQL_bind(const std::string &query, std::list<RetroBind*>& blobs);
-
-    public:
-
-        char* data;
-        uint32_t length;
-        uint32_t index;
-    };
-
-    bool execSQL_bind_blobs(const std::string &query, std::list<RetroDbBlob>& blobs);
 
 private:
 

libretroshare/src/util/rsdbbind.cc

@@ -0,0 +1,73 @@
+/*
+ * RetroShare : RetroDb functionality
+ *
+ * Copyright 2013 Christopher Evi-Parker
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License Version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA.
+ *
+ * Please report all bugs and problems to "retroshare@lunamutt.com".
+ *
+ */
+
+#include "rsdbbind.h"
+
+RsDoubleBind::RsDoubleBind(double value, int index)
+ : RetroBind(DOUBLE, index), mValue(value) {}
+
+RsStringBind::RsStringBind(const std::string& value, int index)
+ : RetroBind(STRING, index), mValue(value) {}
+
+RsInt32Bind::RsInt32Bind(int32_t value, int index)
+ : RetroBind(INT32, index), mValue(value) {}
+
+RsInt64bind::RsInt64bind(int64_t value, int index)
+ : RetroBind(INT64, index), mValue(value) {}
+
+RsBoolBind::RsBoolBind(bool value, int index)
+ : RetroBind(BOOL, index), mValue(value) {}
+
+RsBlobBind::RsBlobBind(char* data, uint32_t dataLen, int index)
+ : RetroBind(BLOB, index), mData(data), mDataLen(dataLen) {}
+
+bool RsDoubleBind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_double(stm, getIndex(), mValue));
+}
+
+bool RsStringBind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_text(stm, getIndex(), mValue.c_str(), mValue.size(), SQLITE_TRANSIENT));
+}
+
+bool RsInt32Bind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_int(stm, getIndex(), mValue));
+}
+
+bool RsInt64bind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_int64(stm, getIndex(), mValue));
+}
+
+bool RsBoolBind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_int(stm, getIndex(), mValue ? 1 : 0));
+}
+
+bool RsBlobBind::bind(sqlite3_stmt* const stm) const
+{
+	return (SQLITE_OK == sqlite3_bind_blob(stm, getIndex(), mData, mDataLen, SQLITE_TRANSIENT));
+}
+

libretroshare/src/util/rsdbbind.h

@@ -0,0 +1,98 @@
+#ifndef RSDBBIND_H_
+#define RSDBBIND_H_
+
+/*
+ * RetroShare : RetroDb functionality
+ *
+ * Copyright 2013 Christopher Evi-Parker
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License Version 2 as published by the Free Software Foundation.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA.
+ *
+ * Please report all bugs and problems to "retroshare@lunamutt.com".
+ *
+ */
+
+#include <string>
+#include <inttypes.h>
+#include <sqlite3.h>
+
+class RetroBind
+{
+public:
+
+	enum BindType { BLOB=1, STRING, INT32, INT64, DOUBLE, BOOL } ;
+	RetroBind(const BindType& type, int index) : mIndex(index), mType(type) {}
+
+	virtual bool bind(sqlite3_stmt* const stm) const = 0;
+	BindType getType() const { return mType; }
+	inline int getIndex() const { return mIndex;}
+	virtual ~RetroBind() {}
+
+private:
+	int mIndex;
+	BindType mType;
+};
+
+class RsDoubleBind : public RetroBind
+{
+public:
+	RsDoubleBind(double value, int index);
+	bool bind(sqlite3_stmt* const stm) const;
+	double mValue;
+};
+
+class RsStringBind : public RetroBind
+{
+public:
+	RsStringBind(const std::string& value, int index);
+	bool bind(sqlite3_stmt* const stm) const ;
+	std::string mValue;
+};
+
+class RsInt32Bind : public RetroBind
+{
+public:
+	RsInt32Bind(int32_t value, int index);
+	bool bind(sqlite3_stmt* const stm) const ;
+	int32_t mValue;
+};
+
+class RsInt64bind : public RetroBind
+{
+public:
+	RsInt64bind(int64_t value, int index);
+	bool bind(sqlite3_stmt* const stm) const ;
+	int64_t mValue;
+};
+
+class RsBoolBind : public RetroBind
+{
+public:
+	RsBoolBind(bool value, int index);
+	bool bind(sqlite3_stmt* const stm) const ;
+	bool mValue;
+};
+
+class RsBlobBind : public RetroBind
+{
+public:
+	RsBlobBind(char* data, uint32_t dataLen, int index);
+	bool bind(sqlite3_stmt* const stm) const;
+	char* mData;
+	uint32_t mDataLen;
+};
+
+
+#endif /* RSDBBIND_H_ */