Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security issue #177

Merged
merged 5 commits into from
Aug 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ const socket = require('socket.io')
const multer = require('multer')
const bodyParser = require('body-parser')
const cors = require('cors')
const helmet = require('helmet')
const hpp = require('hpp')
var winston = require('./config/winston')
const rateLimiter = require('./app/middleware/rateLimiter')
vaibhavdaren marked this conversation as resolved.
Show resolved Hide resolved
const sanitizer = require('./app/middleware/sanitise')
const fileConstants = require('./config/fileHandlingConstants')

const indexRouter = require('./app/routes/index')
Expand All @@ -31,6 +35,7 @@ const server = require('http').Server(app)
app.use(cors())

app.use(bodyParser.json({ limit: '200mb' }))
app.use(cookieParser())
app.use(bodyParser.urlencoded(fileConstants.fileParameters))

const memoryStorage = multer.memoryStorage()
Expand Down Expand Up @@ -72,6 +77,23 @@ app.use((req, res, next) => {
next()
})

// TO PREVENT DOS ATTACK AND RATE LIMITER
app.use(rateLimiter.customRateLimiter)

// TO PREVENT XSS ATTACK
app.use(sanitizer.cleanBody)
app.use(helmet())

// TO PREVENT CLICK JACKING
app.use((req, res, next) => {
res.append('X-Frame-Options', 'Deny')
res.set('Content-Security-Policy', "frame-ancestors 'none';")
next()
})

// TO PREVENT THE QUERY PARAMETER POLLUTION
app.use(hpp())

app.use('/notification', notificationRouter)
app.use('/', indexRouter)
app.use('/auth', authRouter)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ const activityHelper = require('../utils/activity-helper')

module.exports = {
authenticateUser: async (req, res, next) => {
const email = req.body.email
const password = req.body.password
const email = escape(req.body.email)
const password = escape(req.body.password)
try {
const user = await User.findByCredentials(email, password)
const token = await user.generateAuthToken()
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/organization.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ module.exports = {
notification.content = `${orgData.name} is created!`
notification.tag = TAGS.NEW
notificationHelper.addToNotificationForAll(req, res, notification, next)
activityTracker.addToRedis(req, res, next, collectionTypes.ORG, org._id)
// activityTracker.addToRedis(req, res, next, collectionTypes.ORG, org._id)
return res.status(HttpStatus.CREATED).json({ orgData })
} catch (error) {
HANDLER.handleError(res, error)
Expand Down
26 changes: 25 additions & 1 deletion app/controllers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ module.exports = {
// create redis db for activity for the user
const activity = new Activity({ userId: data._id })
await activity.save()

// hide password
user.password = undefined
return res.status(HttpStatus.CREATED).json({ user: user, token: token })
} catch (error) {
return res.status(HttpStatus.NOT_ACCEPTABLE).json({ error: error })
Expand Down Expand Up @@ -81,6 +82,9 @@ module.exports = {
if (!user) {
return res.status(HttpStatus.NOT_FOUND).json({ msg: 'No such user exist!' })
}
// hide password and tokens
user.password = undefined
user.tokens = []
return res.status(HttpStatus.OK).json({ user })
} catch (error) {
HANDLER.handleError(res, error)
Expand All @@ -94,6 +98,7 @@ module.exports = {
'phone',
'info',
'about',
'socialMedia',
'isDeactivated'
]
// added control as per org settings
Expand All @@ -118,6 +123,9 @@ module.exports = {
user[update] = req.body[update]
})
await user.save()
// hide password and tokens
user.password = undefined
user.tokens = []
return res.status(HttpStatus.OK).json({ data: user })
} catch (error) {
return res.status(HttpStatus.BAD_REQUEST).json({ error })
Expand Down Expand Up @@ -235,6 +243,7 @@ module.exports = {
const inviteLink = `${req.protocol}://${req.get('host')}/user/invite/${token}`
return res.status(HttpStatus.OK).json({ inviteLink: inviteLink })
} catch (error) {
console.log('error in req', error)
HANDLER.handleError(res, error)
}
},
Expand Down Expand Up @@ -308,6 +317,9 @@ module.exports = {
.populate('followers', ['name.firstName', 'name.lastName', 'info.about.designation', '_id', 'isAdmin'])
.populate('blocked', ['name.firstName', 'name.lastName', 'info.about.designation', '_id', 'isAdmin'])
.exec()
// hide password and tokens
userData.password = undefined
userData.tokens = []
return res.status(HttpStatus.OK).json({ user: userData })
} catch (error) {
HANDLER.handleError(res, error)
Expand Down Expand Up @@ -358,6 +370,9 @@ module.exports = {
.populate('followers', ['name.firstName', 'name.lastName', 'info.about.designation', '_id', 'isAdmin'])
.populate('blocked', ['name.firstName', 'name.lastName', 'info.about.designation', '_id', 'isAdmin'])
.exec()
// hide password and tokens
userData.password = undefined
userData.tokens = []
return res.status(HttpStatus.OK).json({ user: userData })
} catch (error) {
HANDLER.handleError(res, error)
Expand Down Expand Up @@ -404,6 +419,9 @@ module.exports = {
if (unblockIndex !== -1) {
user.blocked.splice(unblockIndex, 1)
await user.save()
// hide password and tokens
user.password = undefined
user.tokens = []
return res.status(HttpStatus.OK).json({ user })
}
return res.status(HttpStatus.NOT_FOUND).json({ user })
Expand Down Expand Up @@ -441,6 +459,9 @@ module.exports = {
}
user.isRemoved = true
await user.save()
// hide password and tokens
user.password = undefined
user.tokens = []
return res.status(HttpStatus.OK).json({ user })
} catch (error) {
HANDLER.handleError(res, error)
Expand All @@ -451,6 +472,9 @@ module.exports = {
try {
req.user.isActivated = !req.user.isActivated
const user = await req.user.save()
// hide password and tokens
user.password = undefined
user.tokens = []
return res.status(HttpStatus.OK).json({ user })
} catch (error) {
HANDLER.handleError(error)
Expand Down
81 changes: 81 additions & 0 deletions app/middleware/rateLimiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
const redis = require('../../config/redis')
const redisClient = redis.redisClient
const moment = require('moment')
const WINDOW_SIZE_IN_HOURS = 24
const MAX_WINDOW_REQUEST_COUNT = process.env.NODE_ENV === 'testing' ? 20 : 100
const WINDOW_LOG_INTERVAL_IN_HOURS = 1

module.exports = {
customRateLimiter: (req, res, next) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a testcase for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sure

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate limiter is working tried to send more than 100 requests then it's sending this responses:
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Increased no of requests in testing env to 500 and in development or prod 200 no of requests

try {
// check if redis exists
if (!redisClient) {
throw new Error('RedisClient not found on the server')
}
// if exists check if request made earlier from same ip
redisClient.get(req.ip, (err, reply) => {
if (err) {
console.log('Error in fetching data from redis', err)
}
const currentRequestTime = moment()
// if no reply from redis then store the users request to the server in redis
if (reply === null || reply === undefined) {
const newRecord = []
const info = {
requestTimeStamp: currentRequestTime.unix(),
requestCount: 1
}
newRecord.unshift(info)
// set to redis => ip => [{ requestTimeStamp, requestCount }]
redisClient.set(req.ip, JSON.stringify(newRecord))
next()
} else {
// if record is found, parse it's value and calculate number of requests users has made within the last window
const data = JSON.parse(reply)

const windowStartTimestamp = moment()
.subtract(WINDOW_SIZE_IN_HOURS, 'hours')
.unix()

const requestsWithinWindow = data.filter(entry => {
return entry.requestTimeStamp > windowStartTimestamp
})

const totalWindowRequestsCount = requestsWithinWindow.reduce((accumulator, entry) => {
return accumulator + entry.requestCount
}, 0)

// if number of requests made is greater than or equal to the desired maximum, return error
if (totalWindowRequestsCount >= MAX_WINDOW_REQUEST_COUNT) {
return res.status(429).json({
error: `You have exceeded the ${MAX_WINDOW_REQUEST_COUNT} requests in ${WINDOW_SIZE_IN_HOURS} hrs limit!`
})
} else {
// if number of requests made is less than allowed maximum, log new entry
const lastRequestLog = data[data.length - 1]
const potentialCurrentWindowIntervalStartTimeStamp = currentRequestTime
.subtract(WINDOW_LOG_INTERVAL_IN_HOURS, 'hours')
.unix()

// if interval has not passed since last request log, increment counter
if (lastRequestLog.requestTimeStamp > potentialCurrentWindowIntervalStartTimeStamp) {
lastRequestLog.requestCount++
data[data.length - 1] = lastRequestLog
} else {
// if interval has passed, log new entry for current user and timestamp
data.unshift({
requestTimeStamp: currentRequestTime.unix(),
requestCount: 1
})
}
redisClient.set(req.ip, JSON.stringify(data))
next()
}
}
})
} catch (error) {
console.log('Error in rateLimiter', error)
next(error)
}
}
}
7 changes: 7 additions & 0 deletions app/middleware/sanitise.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const sanitize = require('mongo-sanitize')
module.exports = {
cleanBody: (req, res, next) => {
req.body = sanitize(req.body)
next()
}
}
12 changes: 4 additions & 8 deletions app/utils/activity-helper.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,7 @@
const Activity = require('../models/Activity')
const redis = require('../../config/redis')
var redisClient = redis.redisClient
var activityElement = {
route: '',
method: '',
collectionType: '',
id: '',
timestamp: ''
}
var activityElement = {}

module.exports = {
addToRedis: async (req, res, next, collection, objectId) => {
Expand Down Expand Up @@ -48,7 +42,9 @@ module.exports = {
data.activity.unshift(activityElement)
}

await data.save()
if (data !== null || data !== undefined) {
await data.save()
}
console.log('Activity saved to db ', data)

// clear data from redis
Expand Down
Loading