fix: only allow numbers as scores (#7356)

* zadd score checks

* fix: only allow numbers as scores

* fix: convert values to strings

src/database/mongo/helpers.js

@@ -46,9 +46,5 @@ helpers.deserializeData = function (data) {
 };
 
 helpers.valueToString = function (value) {
-	if (value === null || value === undefined) {
-		return value;
-	}
-
-	return value.toString();
+	return String(value);
 };

src/database/mongo/sorted/add.js

@@ -2,6 +2,7 @@
 
 module.exports = function (db, module) {
 	var helpers = module.helpers.mongo;
+	var utils = require('../../../utils');
 
 	module.sortedSetAdd = function (key, score, value, callback) {
 		callback = callback || helpers.noop;
@@ -11,7 +12,9 @@ module.exports = function (db, module) {
 		if (Array.isArray(score) && Array.isArray(value)) {
 			return sortedSetAddBulk(key, score, value, callback);
 		}
-
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
+		}
 		value = helpers.valueToString(value);
 
 		db.collection('objects').updateOne({ _key: key, value: value }, { $set: { score: parseFloat(score) } }, { upsert: true, w: 1 }, function (err) {
@@ -29,7 +32,11 @@ module.exports = function (db, module) {
 		if (scores.length !== values.length) {
 			return callback(new Error('[[error:invalid-data]]'));
 		}
-
+		for (let i = 0; i < scores.length; i += 1) {
+			if (!utils.isNumber(scores[i])) {
+				return setImmediate(callback, new Error('[[error:invalid-score, ' + scores[i] + ']]'));
+			}
+		}
 		values = values.map(helpers.valueToString);
 
 		var bulk = db.collection('objects').initializeUnorderedBulkOp();
@@ -48,6 +55,9 @@ module.exports = function (db, module) {
 		if (!Array.isArray(keys) || !keys.length) {
 			return callback();
 		}
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
+		}
 		value = helpers.valueToString(value);
 
 		var bulk = db.collection('objects').initializeUnorderedBulkOp();

src/database/postgres/helpers.js

@@ -3,11 +3,7 @@
 var helpers = {};
 
 helpers.valueToString = function (value) {
-	if (value === null || value === undefined) {
-		return value;
-	}
-
-	return value.toString();
+	return String(value);
 };
 
 helpers.removeDuplicateValues = function (values) {

src/database/postgres/sorted/add.js

@@ -4,6 +4,7 @@ var async = require('async');
 
 module.exports = function (db, module) {
 	var helpers = module.helpers.postgres;
+	var utils = require('../../../utils');
 
 	module.sortedSetAdd = function (key, score, value, callback) {
 		callback = callback || helpers.noop;
@@ -15,7 +16,9 @@ module.exports = function (db, module) {
 		if (Array.isArray(score) && Array.isArray(value)) {
 			return sortedSetAddBulk(key, score, value, callback);
 		}
-
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
+		}
 		value = helpers.valueToString(value);
 		score = parseFloat(score);
 
@@ -46,7 +49,11 @@ VALUES ($1::TEXT, $2::TEXT, $3::NUMERIC)
 		if (scores.length !== values.length) {
 			return callback(new Error('[[error:invalid-data]]'));
 		}
-
+		for (let i = 0; i < scores.length; i += 1) {
+			if (!utils.isNumber(scores[i])) {
+				return setImmediate(callback, new Error('[[error:invalid-score, ' + scores[i] + ']]'));
+			}
+		}
 		values = values.map(helpers.valueToString);
 		scores = scores.map(function (score) {
 			return parseFloat(score);
@@ -81,7 +88,9 @@ SELECT $1::TEXT, v, s
 		if (!Array.isArray(keys) || !keys.length) {
 			return callback();
 		}
-
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
+		}
 		value = helpers.valueToString(value);
 		score = parseFloat(score);
 

src/database/redis/sorted/add.js

@@ -1,6 +1,8 @@
 'use strict';
 
 module.exports = function (redisClient, module) {
+	const utils = require('../../../utils');
+
 	module.sortedSetAdd = function (key, score, value, callback) {
 		callback = callback || function () {};
 		if (!key) {
@@ -9,7 +11,10 @@ module.exports = function (redisClient, module) {
 		if (Array.isArray(score) && Array.isArray(value)) {
 			return sortedSetAddMulti(key, score, value, callback);
 		}
-		redisClient.zadd(key, score, value, function (err) {
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
+		}
+		redisClient.zadd(key, score, String(value), function (err) {
 			callback(err);
 		});
 	};
@@ -22,11 +27,15 @@ module.exports = function (redisClient, module) {
 		if (scores.length !== values.length) {
 			return callback(new Error('[[error:invalid-data]]'));
 		}
-
+		for (let i = 0; i < scores.length; i += 1) {
+			if (!utils.isNumber(scores[i])) {
+				return setImmediate(callback, new Error('[[error:invalid-score, ' + scores[i] + ']]'));
+			}
+		}
 		var args = [key];
 
 		for (var i = 0; i < scores.length; i += 1) {
-			args.push(scores[i], values[i]);
+			args.push(scores[i], String(values[i]));
 		}
 
 		redisClient.zadd(args, function (err) {
@@ -37,13 +46,16 @@ module.exports = function (redisClient, module) {
 	module.sortedSetsAdd = function (keys, score, value, callback) {
 		callback = callback || function () {};
 		if (!Array.isArray(keys) || !keys.length) {
-			return callback();
+			return setImmediate(callback);
+		}
+		if (!utils.isNumber(score)) {
+			return setImmediate(callback, new Error('[[error:invalid-score, ' + score + ']]'));
 		}
 		var batch = redisClient.batch();
 
 		for (var i = 0; i < keys.length; i += 1) {
 			if (keys[i]) {
-				batch.zadd(keys[i], score, value);
+				batch.zadd(keys[i], score, String(value));
 			}
 		}
 

test/database/sorted.js

@@ -57,6 +57,31 @@ describe('Sorted Set methods', function () {
 				});
 			});
 		});
+
+		it('should error if score is null', function (done) {
+			db.sortedSetAdd('errorScore', null, 'value1', function (err) {
+				assert.equal(err.message, '[[error:invalid-score, null]]');
+				done();
+			});
+		});
+
+		it('should error if any score is undefined', function (done) {
+			db.sortedSetAdd('errorScore', [1, undefined], ['value1', 'value2'], function (err) {
+				assert.equal(err.message, '[[error:invalid-score, undefined]]');
+				done();
+			});
+		});
+
+		it('should add null value as `null` string', function (done) {
+			db.sortedSetAdd('nullValueZSet', 1, null, function (err) {
+				assert.ifError(err);
+				db.getSortedSetRange('nullValueZSet', 0, -1, function (err, values) {
+					assert.ifError(err);
+					assert.strictEqual(values[0], 'null');
+					done();
+				});
+			});
+		});
 	});
 
 	describe('sortedSetsAdd()', function () {
@@ -67,6 +92,14 @@ describe('Sorted Set methods', function () {
 				done();
 			});
 		});
+
+
+		it('should error if score is null', function (done) {
+			db.sortedSetsAdd(['sorted1', 'sorted2'], null, 'value1', function (err) {
+				assert.equal(err.message, '[[error:invalid-score, null]]');
+				done();
+			});
+		});
 	});
 
 	describe('getSortedSetRange()', function () {