From c5c559ffe1728b430c8bd7a1bf2297ee245a383c Mon Sep 17 00:00:00 2001 From: chrisparker126 Date: Sat, 20 Apr 2013 13:39:02 +0000 Subject: [PATCH] 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 | 3 + libretroshare/src/serialiser/rsgxsitems.cc | 14 +- libretroshare/src/util/contentvalue.h | 1 + libretroshare/src/util/retrodb.cc | 333 +++++++++++---------- libretroshare/src/util/retrodb.h | 30 +- libretroshare/src/util/rsdbbind.cc | 73 +++++ libretroshare/src/util/rsdbbind.h | 98 ++++++ 7 files changed, 372 insertions(+), 180 deletions(-) create mode 100644 libretroshare/src/util/rsdbbind.cc create mode 100644 libretroshare/src/util/rsdbbind.h diff --git a/libretroshare/src/libretroshare.pro b/libretroshare/src/libretroshare.pro index 7513e9670..6e1b13c4d 100644 --- a/libretroshare/src/libretroshare.pro +++ b/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 \ diff --git a/libretroshare/src/serialiser/rsgxsitems.cc b/libretroshare/src/serialiser/rsgxsitems.cc index 00fc2b100..8432d6e9a 100644 --- a/libretroshare/src/serialiser/rsgxsitems.cc +++ b/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->mSignFlags = rGxsMeta.mSignFlags; - this->mCircleId = rGxsMeta.mCircleId; - this->mCircleType = rGxsMeta.mCircleType; - this->mInternalCircle = rGxsMeta.mInternalCircle; - this->mOriginator = rGxsMeta.mOriginator; + this->mServiceString = rGxsMeta.mServiceString; + this->mSignFlags = rGxsMeta.mSignFlags; + this->mCircleId = rGxsMeta.mCircleId; + this->mCircleType = rGxsMeta.mCircleType; + this->mInternalCircle = rGxsMeta.mInternalCircle; + this->mOriginator = rGxsMeta.mOriginator; } diff --git a/libretroshare/src/util/contentvalue.h b/libretroshare/src/util/contentvalue.h index 50b12af48..8d2d97e4e 100644 --- a/libretroshare/src/util/contentvalue.h +++ b/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 */ diff --git a/libretroshare/src/util/retrodb.cc b/libretroshare/src/util/retrodb.cc index fead5a740..e3e5ba9ed 100644 --- a/libretroshare/src/util/retrodb.cc +++ b/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 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::ostringstream oStrStream; - uint32_t index = 0; - std::list 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(""); - } - + std::string qValues; + std::list paramBindings; + buildInsertQueryValue(keyTypeMap, cv, qValues, paramBindings); // 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 &blobs){ +bool RetroDb::execSQL_bind(const std::string &query, std::list ¶mBindings){ // prepare statement sqlite3_stmt* stm = NULL; @@ -328,11 +248,19 @@ bool RetroDb::execSQL_bind_blobs(const std::string &query, std::list::iterator lit = blobs.begin(); + std::list::iterator lit = paramBindings.begin(); - for(; lit != blobs.end(); lit++){ - const RetroDbBlob& b = *lit; - sqlite3_bind_blob(stm, b.index, b.data, b.length, NULL); + for(; lit != paramBindings.end(); lit++){ + RetroBind* rb = *lit; + + 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 keyTypeMap, + const ContentValue& cv, std::string& parameter, + std::list& paramBindings) +{ + std::map::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 keyTypeMap, + const ContentValue& cv, std::string& parameter, + std::list& paramBindings) +{ + std::map::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::ostringstream oStrStream; - - 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(""); - } + std::string qValues; + std::list paramBindings; + buildUpdateQueryValue(keyTypeMap, cv, qValues, paramBindings); 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) diff --git a/libretroshare/src/util/retrodb.h b/libretroshare/src/util/retrodb.h index 38706547f..0ee96e54e 100644 --- a/libretroshare/src/util/retrodb.h +++ b/libretroshare/src/util/retrodb.h @@ -30,9 +30,12 @@ #include #include #include +#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 keyMap, const ContentValue& cv, + std::string& parameter, std::list& paramBindings); + + /*! + * Build the "VALUE" part of an insertiong sql query + * @param parameter contains place holder query + * @param paramBindings + */ + void buildUpdateQueryValue(const std::map keyMap, const ContentValue& cv, + std::string& parameter, std::list& paramBindings); public: static const int OPEN_READONLY; @@ -149,16 +168,7 @@ public: private: - class RetroDbBlob{ - - public: - - char* data; - uint32_t length; - uint32_t index; - }; - - bool execSQL_bind_blobs(const std::string &query, std::list& blobs); + bool execSQL_bind(const std::string &query, std::list& blobs); private: diff --git a/libretroshare/src/util/rsdbbind.cc b/libretroshare/src/util/rsdbbind.cc new file mode 100644 index 000000000..31e548ebd --- /dev/null +++ b/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)); +} + diff --git a/libretroshare/src/util/rsdbbind.h b/libretroshare/src/util/rsdbbind.h new file mode 100644 index 000000000..8d4a7f44d --- /dev/null +++ b/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 +#include +#include + +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_ */