From a7a3f3619b070ce000c91321f13eb38e561fdc4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?= Date: Thu, 30 Nov 2017 14:24:13 -0500 Subject: [PATCH] dont allow login with invalid ip, escape ip display on user/info page --- src/controllers/authentication.js | 39 +++++++++++++++++-------------- src/meta/blacklist.js | 19 +++++++-------- src/user/admin.js | 34 +++++++++++++++++++++------ src/user/auth.js | 16 ++++++++----- test/authentication.js | 30 ++++++++++++++++++++++++ test/blacklist.js | 20 +++++----------- 6 files changed, 103 insertions(+), 55 deletions(-) diff --git a/src/controllers/authentication.js b/src/controllers/authentication.js index 73eb6be287..c547ada0dd 100644 --- a/src/controllers/authentication.js +++ b/src/controllers/authentication.js @@ -6,6 +6,7 @@ var passport = require('passport'); var nconf = require('nconf'); var validator = require('validator'); var _ = require('lodash'); +var ipaddr = require('ipaddr.js'); var db = require('../database'); var meta = require('../meta'); @@ -289,26 +290,30 @@ authenticationController.doLogin = function (req, uid, callback) { authenticationController.onSuccessfulLogin = function (req, uid, callback) { var uuid = utils.generateUUID(); - req.session.meta = {}; - - delete req.session.forceLogin; - - // Associate IP used during login with user account - user.logIP(uid, req.ip); - req.session.meta.ip = req.ip; - - // Associate metadata retrieved via user-agent - req.session.meta = _.extend(req.session.meta, { - uuid: uuid, - datetime: Date.now(), - platform: req.useragent.platform, - browser: req.useragent.browser, - version: req.useragent.version, - }); async.waterfall([ - async.apply(meta.blacklist.test, req.ip), function (next) { + meta.blacklist.test(req.ip, next); + }, + function (next) { + user.logIP(uid, req.ip, next); + }, + function (next) { + req.session.meta = {}; + + delete req.session.forceLogin; + // Associate IP used during login with user account + req.session.meta.ip = req.ip; + + // Associate metadata retrieved via user-agent + req.session.meta = _.extend(req.session.meta, { + uuid: uuid, + datetime: Date.now(), + platform: req.useragent.platform, + browser: req.useragent.browser, + version: req.useragent.version, + }); + async.parallel([ function (next) { user.auth.addSession(uid, req.sessionID, next); diff --git a/src/meta/blacklist.js b/src/meta/blacklist.js index c8d5b6a9fb..2b2f897284 100644 --- a/src/meta/blacklist.js +++ b/src/meta/blacklist.js @@ -68,7 +68,12 @@ Blacklist.test = function (clientIp, callback) { // clientIp = '127.0.15.1:3443'; // IPv4 with port strip port to not fail clientIp = clientIp.split(':').length === 2 ? clientIp.split(':')[0] : clientIp; - var addr = ipaddr.parse(clientIp); + var addr; + try { + addr = ipaddr.parse(clientIp); + } catch (err) { + return callback(err); + } if ( Blacklist._rules.ipv4.indexOf(clientIp) === -1 && // not explicitly specified in ipv4 list @@ -88,11 +93,7 @@ Blacklist.test = function (clientIp, callback) { analytics.increment('blacklist'); } - if (typeof callback === 'function') { - callback(err); - } else { - return !!err; - } + callback(err); }); } else { var err = new Error('[[error:blacklisted-ip]]'); @@ -100,11 +101,7 @@ Blacklist.test = function (clientIp, callback) { analytics.increment('blacklist'); - if (typeof callback === 'function') { - setImmediate(callback, err); - } else { - return true; - } + setImmediate(callback, err); } }; diff --git a/src/user/admin.js b/src/user/admin.js index d463523f89..1b273bca48 100644 --- a/src/user/admin.js +++ b/src/user/admin.js @@ -2,21 +2,41 @@ 'use strict'; var async = require('async'); +var winston = require('winston'); +var validator = require('validator'); + var db = require('../database'); var plugins = require('../plugins'); -var winston = require('winston'); module.exports = function (User) { - User.logIP = function (uid, ip) { + User.logIP = function (uid, ip, callback) { var now = Date.now(); - db.sortedSetAdd('uid:' + uid + ':ip', now, ip || 'Unknown'); - if (ip) { - db.sortedSetAdd('ip:' + ip + ':uid', now, uid); - } + async.waterfall([ + function (next) { + db.sortedSetAdd('uid:' + uid + ':ip', now, ip || 'Unknown', next); + }, + function (next) { + if (ip) { + db.sortedSetAdd('ip:' + ip + ':uid', now, uid, next); + } else { + next(); + } + }, + ], callback); }; User.getIPs = function (uid, stop, callback) { - db.getSortedSetRevRange('uid:' + uid + ':ip', 0, stop, callback); + async.waterfall([ + function (next) { + db.getSortedSetRevRange('uid:' + uid + ':ip', 0, stop, next); + }, + function (ips, next) { + ips = ips.map(function (ip) { + return validator.escape(String(ip)); + }); + next(null, ips); + }, + ], callback); }; User.getUsersCSV = function (callback) { diff --git a/src/user/auth.js b/src/user/auth.js index 1fbc316f18..6d0002939e 100644 --- a/src/user/auth.js +++ b/src/user/auth.js @@ -2,6 +2,7 @@ var async = require('async'); var winston = require('winston'); +var validator = require('validator'); var db = require('../database'); var meta = require('../meta'); var events = require('../events'); @@ -126,12 +127,15 @@ module.exports = function (User) { next(err, sessions); }); }, - ], function (err, sessions) { - callback(err, sessions ? sessions.map(function (sessObj) { - sessObj.meta.datetimeISO = new Date(sessObj.meta.datetime).toISOString(); - return sessObj.meta; - }) : undefined); - }); + function (sessions, next) { + sessions = sessions.map(function (sessObj) { + sessObj.meta.datetimeISO = new Date(sessObj.meta.datetime).toISOString(); + sessObj.meta.ip = validator.escape(String(sessObj.meta.ip)); + return sessObj.meta; + }); + next(null, sessions); + }, + ], callback); }; User.auth.addSession = function (uid, sessionId, callback) { diff --git a/test/authentication.js b/test/authentication.js index bacbcb679a..99512c7af4 100644 --- a/test/authentication.js +++ b/test/authentication.js @@ -235,6 +235,36 @@ describe('authentication', function () { }); }); + it('should fail to login if ip address if invalid', function (done) { + var jar = request.jar(); + request({ + url: nconf.get('url') + '/api/config', + json: true, + jar: jar, + }, function (err, response, body) { + if (err) { + return done(err); + } + + request.post(nconf.get('url') + '/login', { + form: { + username: 'regular', + password: 'regularpwd', + }, + json: true, + jar: jar, + headers: { + 'x-csrf-token': body.csrf_token, + 'x-forwarded-for': '', + }, + }, function (err, response, body) { + assert.ifError(err); + assert.equal(response.statusCode, 500); + done(); + }); + }); + }); + it('should fail to login if user does not exist', function (done) { loginUser('doesnotexist', 'nopassword', function (err, response, body) { assert.ifError(err); diff --git a/test/blacklist.js b/test/blacklist.js index 763c5364f7..9ec1e557d3 100644 --- a/test/blacklist.js +++ b/test/blacklist.js @@ -49,32 +49,24 @@ describe('blacklist', function () { }); }); - it('should pass ip test against blacklist async', function (done) { + it('should pass ip test against blacklist', function (done) { blacklist.test('3.3.3.3', function (err) { assert.ifError(err); done(); }); }); - it('should pass ip test against blacklist sync', function (done) { - assert(!blacklist.test('3.3.3.3')); - done(); - }); - - it('should fail ip test against blacklist async', function (done) { + it('should fail ip test against blacklist', function (done) { blacklist.test('1.1.1.1', function (err) { assert.equal(err.message, '[[error:blacklisted-ip]]'); done(); }); }); - it('should fail ip test against blacklist sync', function (done) { - assert(blacklist.test('1.1.1.1')); - done(); - }); - it('should pass ip test and not crash with ipv6 address', function (done) { - assert(!blacklist.test('2001:db8:85a3:0:0:8a2e:370:7334')); - done(); + blacklist.test('2001:db8:85a3:0:0:8a2e:370:7334', function (err) { + assert.ifError(err); + done(); + }); }); });