CVE-2017-20171: Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection') (CWE-89)
value: A vulnerability classified as critical has been found in PrivateSky apersistence. This affects an unknown part of the file db/sql/mysqlUtils.js. The manipulation leads to sql injection. The identifier of the patch is 954425f61634b556fe644837a592a5b8fcfca068. It is recommended to apply a patch to fix this issue. The identifier VDB-218457 was assigned to this vulnerability.
CWE description: The product constructs all or part of an SQL command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended SQL command when it is sent to a downstream component. Without sufficient removal or quoting of SQL syntax in user-controllable inputs, the generated SQL query can cause those inputs to be interpreted as SQL instead of ordinary user data.
CVSS v3 base score: 9.8
Published: 2023-01-17T23:15Z
Repository: https://github.com/PrivateSky/apersistence
Language: JavaScript
Files changed: db/sql/MySqlPersistence.js, db/sql/mysqlUtils.js, lib/BasicStrategy.js, lib/ModelDescription.js, lib/basicSQLTypes.js, test/fillLazyTest.js
Commit date: 2017-04-10 09:29:58 +0300
Commit message: fixed SQL injection bug and changed type boolean from bit to tinyint

Diff: diff --git a/db/sql/MySqlPersistence.js b/db/sql/MySqlPersistence.js
index 7b7c84b..79a3238 100755
--- a/db/sql/MySqlPersistence.js
+++ b/db/sql/MySqlPersistence.js
@@ -19,18 +19,17 @@ function sqlPersistenceStrategy(mysqlPool) {
 
         function validate(tableStructure){
 
             var validModel = true;
             var model = new modelUtil.ModelDescription(typeName,description,self);
-
+            
             tableStructure[0].forEach(function(column){
-                column['Type'] = column['Type'].split('(')[0];   //ignore size specifications such as INT(10)
+                column['Type'] = column['Type'].split('(')[0];   //ignore size specifications such as INT(10) ... not neccesarily reccomender
             });
 
             model.persistentProperties.some(function(modelProperty){
                 var expectedDbType = self.getDatabaseType(model.getFieldType(modelProperty));
-
                 if(expectedDbType === undefined){
                     validModel = false;
                     return true;
                 }
 
@@ -95,12 +94,11 @@ function sqlPersistenceStrategy(mysqlPool) {
         mysqlPool.query(query,function(err,result){
             if(err){
                 callback(err);
             }else{
                 var model = modelUtil.getModel(typeName);
-                
-                var deserialized_id = modelUtil.deserialiseField(typeName,model.getPKField(),serialized_id,self)
+                var deserialized_id = modelUtil.deserialiseField(typeName,model.getPKField(),serialized_id,self);
                 var retObj = createRawObject(typeName, deserialized_id);
                 if (result.length>0) {
                     modelUtil.load(retObj, result[0], self);
                 }
                 self.cache[deserialized_id] = retObj;
diff --git a/db/sql/mysqlUtils.js b/db/sql/mysqlUtils.js
index d24144d..ddabcc2 100755
--- a/db/sql/mysqlUtils.js
+++ b/db/sql/mysqlUtils.js
@@ -1,10 +1,10 @@
 /**
  * Created by ctalmacel on 12/21/15.
  */
 
-
+var mysql = require('mysql');
 var Q = require('q');
 var modelUtil = require("../../lib/ModelDescription.js");
 
 exports.createTable= function(persistenceStrategy,tableName,model){
     var query = 'CREATE TABLE IF NOT EXISTS '+tableName+'(';
@@ -56,18 +56,11 @@ exports.insertRow = function(tableName,serializedData){
     query += ") VALUES (";
 
 
 
     for(var field in serializedData){
-        
-        if(model.getFieldDescription(field).type === 'boolean') {
-            query+=' b\''+serializedData[field]+'\',';
-        }else{
-            query+=' \''+serializedData[field]+'\',';
-        }
-
-
+        query+= mysql.escape(serializedData[field])+',';
     }
     query = query.slice(0, -1);
     query+=');';
     return query;
 };
diff --git a/lib/BasicStrategy.js b/lib/BasicStrategy.js
index c3d46e1..bf035ea 100755
--- a/lib/BasicStrategy.js
+++ b/lib/BasicStrategy.js
@@ -12,11 +12,11 @@ function BasicStrategy(){
         typeConverterRegistryToDatabase[typeName] = toDatabase;
 
         if(dbType !== undefined) {
 
             if(dbType.indexOf(')') != -1){
-                dbType = dbType.slice(dbType.indexOf('('));
+                dbType = dbType.slice(0,dbType.indexOf('('));
             }
             typeToDbTypeCorrespondence[typeName] = dbType;
             dbTypeToTypeCorrespondence[dbType] = typeName;
         }
     }
diff --git a/lib/ModelDescription.js b/lib/ModelDescription.js
index 180559a..d26dedc 100755
--- a/lib/ModelDescription.js
+++ b/lib/ModelDescription.js
@@ -130,11 +130,10 @@ function ModelDescription(typeName, description, strategy){
             res[v] = template[v];
         }
 
         // throw erros if trying to access lazy fields that are not loaded or setting fields #this is not OOP:)
         self.transientProperties.forEach(function(field){
-
             Object.defineProperty(res,field,{
                 get:function(field){
                     return null;
                 },
                 set:function(){
@@ -247,11 +246,11 @@ function convertTo(modelName, fieldName,value, strategy){
 
 exports.load = function( rawObject, from , strategy){
     var rawModel = models[rawObject.__meta.typeName];
     var props = rawModel.persistentProperties;
     props.forEach(function(p){
-        if(from[p] || from[p]===false) {
+        if(from.hasOwnProperty(p)) {
             var value = convertFrom(strategy, rawObject.__meta.typeName, p, from[p]);
             rawObject[p] = value;
             rawObject.__meta.savedValues[p] = value;
         }
     });
diff --git a/lib/basicSQLTypes.js b/lib/basicSQLTypes.js
index b981d57..9dfff9a 100755
--- a/lib/basicSQLTypes.js
+++ b/lib/basicSQLTypes.js
@@ -43,27 +43,26 @@ exports.registerTypeConverters = function(persistence){
         "float"
     )
 
     persistence.registerConverter('boolean',
         function(value){
-            
-            if(value[0] != '0') {
+            if(value == 1) {
                 return true;
             }
             else {
                 return false;
             }
         },
         function(value){
-            if(value){
-                return "1"
+            if(value == true){
+                return 1
             }
             else{
-                return "0"
+                return 0
             }
         },
-        "bit"
+        "tinyint(1)"
     );
 
     persistence.registerConverter('date',
         function(value){
             return value.toISOString().substring(0,10);
@@ -98,16 +97,10 @@ exports.registerTypeConverters = function(persistence){
         },
         function(arrayOfStuff, typeDescription){
             if(arrayOfStuff == "null"){
                 return null;
             }
-            if(arrayOfStuff.length>0&&arrayOfStuff[0].__meta){
-                //the array is of lazy objects and was filled
-                arrayOfStuff = arrayOfStuff.map(function(lazyLoadedObject){
-                    return lazyLoadedObject.__meta.getPK()
-                })
-            }
 
             return JSON.stringify(arrayOfStuff)
         },
         'blob'
     )
diff --git a/test/fillLazyTest.js b/test/fillLazyTest.js
index ff23a03..51537f5 100644
--- a/test/fillLazyTest.js
+++ b/test/fillLazyTest.js
@@ -107,10 +107,10 @@ assert.steps("Load lazy objects test",[
                         next();
                     });
                 });
             })
         })
-    }]);
+    }],1000);
 
 
 
 

Vulnerable code: 
                column['Type'] = column['Type'].split('(')[0];   //ignore size specifications such as INT(10)

                
                var deserialized_id = modelUtil.deserialiseField(typeName,model.getPKField(),serialized_id,self)

        
        if(model.getFieldDescription(field).type === 'boolean') {
            query+=' b\''+serializedData[field]+'\',';
        }else{
            query+=' \''+serializedData[field]+'\',';
        }


                dbType = dbType.slice(dbType.indexOf('('));

        if(from[p] || from[p]===false) {
            
            if(value[0] != '0') {
            if(value){
                return "1"
                return "0"
        "bit"
            if(arrayOfStuff.length>0&&arrayOfStuff[0].__meta){
                //the array is of lazy objects and was filled
                arrayOfStuff = arrayOfStuff.map(function(lazyLoadedObject){
                    return lazyLoadedObject.__meta.getPK()
                })
            }
    }]);
Fixed code:             
                column['Type'] = column['Type'].split('(')[0];   //ignore size specifications such as INT(10) ... not neccesarily reccomender
                var deserialized_id = modelUtil.deserialiseField(typeName,model.getPKField(),serialized_id,self);
var mysql = require('mysql');
        query+= mysql.escape(serializedData[field])+',';
                dbType = dbType.slice(0,dbType.indexOf('('));
        if(from.hasOwnProperty(p)) {
            if(value == 1) {
            if(value == true){
                return 1
                return 0
        "tinyint(1)"
    }],1000);