Skip to content

Commit

Permalink
fix: rate limit store requests and sanitize path (#357)
Browse files Browse the repository at this point in the history
* fix: rate limit store requests and sanitize path

* fix: lint issues
  • Loading branch information
robertsLando authored Jan 26, 2021
1 parent 58d8499 commit d76a8c2
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 28 deletions.
4 changes: 0 additions & 4 deletions .github/bot-scripts/renameCommitCheck.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@ async function main(param) {
repo: context.repo.repo,
};

const request = {
...options,
pull_number: context.issue.number,
};
const { data: commits } = await github.pulls.listCommits({
...options,
pull_number: context.issue.number,
Expand Down
54 changes: 33 additions & 21 deletions app.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,19 @@ const path = require('path')
const { storeDir } = reqlib('config/app.js')
const renderIndex = reqlib('/lib/renderIndex')
const archiver = require('archiver')
const rateLimit = require('express-rate-limit')

const storeLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100,
handler: function (req, res) {
res.json({
success: false,
message:
'Request limit reached. You can make only 100 reqests every 15 minutes'
})
}
})

const socketManager = new SocketManager()

Expand All @@ -38,6 +51,17 @@ function start (server) {
startGateway()
}

function sanitizePath (path) {
// remove every ../
path = path.replace(/\.\.\//, '')

if (!path.startsWith(storeDir)) {
throw Error('Path not allowed')
}

return path
}

function setupLogging (settings) {
loggers.setupAll(settings ? settings.gateway : null)
}
Expand Down Expand Up @@ -267,7 +291,7 @@ app.post('/api/importConfig', async function (req, res) {
})

// get config
app.get('/api/store', async function (req, res) {
app.get('/api/store', storeLimiter, async function (req, res) {
try {
async function parseDir (dir) {
const toReturn = []
Expand Down Expand Up @@ -299,13 +323,9 @@ app.get('/api/store', async function (req, res) {
}
})

app.get('/api/store/:path', async function (req, res) {
app.get('/api/store/:path', storeLimiter, async function (req, res) {
try {
const reqPath = req.params.path

if (!reqPath.startsWith(storeDir)) {
throw Error('Path not allowed')
}
const reqPath = sanitizePath(req.params.path)

const stat = await fs.lstat(reqPath)

Expand All @@ -322,13 +342,9 @@ app.get('/api/store/:path', async function (req, res) {
}
})

app.put('/api/store/:path', async function (req, res) {
app.put('/api/store/:path', storeLimiter, async function (req, res) {
try {
const reqPath = req.params.path

if (!reqPath.startsWith(storeDir)) {
throw Error('Path not allowed')
}
const reqPath = sanitizePath(req.params.path)

const stat = await fs.lstat(reqPath)

Expand All @@ -345,13 +361,9 @@ app.put('/api/store/:path', async function (req, res) {
}
})

app.delete('/api/store/:path', async function (req, res) {
app.delete('/api/store/:path', storeLimiter, async function (req, res) {
try {
const reqPath = req.params.path

if (!reqPath.startsWith(storeDir)) {
throw Error('Path not allowed')
}
const reqPath = sanitizePath(req.params.path)

await fs.remove(reqPath)

Expand All @@ -362,7 +374,7 @@ app.delete('/api/store/:path', async function (req, res) {
}
})

app.put('/api/store-multi', async function (req, res) {
app.put('/api/store-multi', storeLimiter, async function (req, res) {
try {
const files = req.body.files || []
for (const f of files) {
Expand All @@ -375,7 +387,7 @@ app.put('/api/store-multi', async function (req, res) {
}
})

app.post('/api/store-multi', function (req, res) {
app.post('/api/store-multi', storeLimiter, function (req, res) {
const files = req.body.files || []

const archive = archiver('zip')
Expand Down
3 changes: 1 addition & 2 deletions lib/ZwaveClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ function driverReady () {
.on('heal network progress', onHealNetworkProgress.bind(this))
// .on('heal network done', onHealNetworkDone.bind(this))

// eslint-disable-next-line no-unused-vars
for (const [nodeId, node] of this.driver.controller.nodes) {
for (const [, node] of this.driver.controller.nodes) {
// node added will not be triggered if the node is in cache
addNode.call(this, node)

Expand Down
5 changes: 5 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
"debug": "^4.3.1",
"ejs": "^3.1.5",
"express": "^4.17.1",
"express-rate-limit": "^5.2.3",
"fs-extra": "^9.1.0",
"jsonfile": "^6.1.0",
"morgan": "~1.10.0",
Expand Down
2 changes: 1 addition & 1 deletion src/modules/ColumnFilterHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class ColumnFilterHelper {
const filter = {}
ColumnFilterHelper.filterProps(colType).forEach(key => {
if (value[key] !== undefined && value[key] !== null) {
if (Array.isArray(value[key]) && !value[key].length > 0) {
if (Array.isArray(value[key]) && value[key].length === 0) {
// Skip empty arrays
return
}
Expand Down

0 comments on commit d76a8c2

Please sign in to comment.