Skip to content
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
20 changes: 20 additions & 0 deletions docs/rules/detect-improper-exception-handling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# detect improper exception handling (detect-improper-exception-handling)

Node.js has the process global object, which is an EventEmitter object that in this case emits uncaught exceptions and brought up to the main event loop. In the event that an uncaught exception is thrown while your application is executing, it will result to it crashing. Thus, it is essential to make a listener for uncaught exceptions uwing the process object. There is no need for importing the process core module as it is nautomatically injected with Node.js.

It is worth noting that this way of exception handling is intended as a last resort. Moreover, this type of event signals that the application is in an undefined state, and resuming the application is strongly discouraged and can cause greater issues.

To not miss any uncaught exception and correctly use it is to do synchronous cleanup of allocated resources such as using handlers, file descriptors, custom loggers and similar before exiting the process with a non-zero exit code. Be aware that in displaying error messages it is recommended to log the necessary details but not as far as leaking detailed information such as stack traces to the user.

### Example
```javascript
process.on("uncaughtException", function(err) {
// clean up allocated resources
// log necessary error details to log files
process.exit(1); // exit the process to avoid unknown state
});
```

[link 1](https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html#handle-uncaughtexception)
[link 2](https://nodejs.org/dist/latest-v16.x/docs/api/process.html#event-uncaughtexception)
[link 3](https://nodejs.dev/learn/error-handling-in-nodejs#catching-uncaught-exceptions)
64 changes: 64 additions & 0 deletions docs/rules/detect-unhandled-async-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# detect unhandled async errors (detect-unhandled-async-errors)

It is impossible to have no errors in your program, even those of the best programmers have one way or another. There are some ways to handle errors in order to find and fix them.

### try-catch method
```javascript
async function userProfile() {
try {
let user = await getUser();
let friendsOfUser = await getFriendsOfUser(userId);
let posts = await getUsersPosts(userId);

showUserProfilePage(user, friendsOfUser, posts);
} catch(e) {
console.log(e);
}
}
```
Your code won't look too good having multiple request handlers each with a try/catch statement.

### .catch method
The .catch method can be used if we want to know which error comes from which async request.

```javascript
async function userProfile() {
let user = await getUser().catch(err => console.error('an error occurred'));
let friendsOfUser = await getFriendsOfUser(userId).catch(err => console.error('an error occurred'));
let posts = await getUsersPosts(userId).catch(err => console.error('an error occurred'));

showUserProfilePage(user, friendsOfUser, posts);
}
```

### using a separate error handling function (Recommended)
With error handler functions. Lessening the need for try-catch blocks and .catch methods.

```javascript
const handle = (promise) => {
return promise
.then(data => ([data, undefined]))
.catch(error => Promise.resolve([undefined, error]));
}

async function userProfile() {
let [user, userErr] = await handle(getUser());

if(userErr) throw new Error('Could not fetch user details');

let [friendsOfUser, friendErr] = await handle(
getFriendsOfUser(userId)
);

if(friendErr) throw new Error('Could not fetch user\'s friends');

let [posts, postErr] = await handle(getUsersPosts(userId));

if(postErr) throw new Error('Could not fetch user\'s posts');

showUserProfilePage(user, friendsOfUser, posts);
}
```

[link 1](https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html#handle-errors-in-asynchronous-calls)
[link 2](https://dev.to/sobiodarlington/better-error-handling-with-async-await-2e5m)
30 changes: 30 additions & 0 deletions docs/rules/detect-unhandled-event-errors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# detect unhandled error events (detect-unhandled-error-events)

When using the EventEmitter, it cannot be denied that errors can occur anywhere in the chain.

Given this code below:
```javascript
const EventEmitter = require('events')
const myEmitter = new EventEmitter()

myEmitter.emit('error', new Error('whoops!'))
// Throws and crashes Node.js
```
Since there is no error event listener called, the error will be thrown and will be treated as an uncaughtException. Once an error is emitted and unhandled, the Node.js application will crash.

There should always be listeners for events at the least. This of course does not count out error events.

```javascript
const EventEmitter = require('events')
const myEmitter = new EventEmitter()
// MUST: include error listener
myEmitter.on('error', (err) => {
console.error('An error occurred')
})
myEmitter.emit('error', new Error('whoops!'))
// Prints: An error occurred
```


[link 1](https://nodejs.org/dist/latest-v16.x/docs/api/events.html#error-events)
[link 2](https://cheatsheetseries.owasp.org/cheatsheets/Nodejs_Security_Cheat_Sheet.html#listen-to-errors-when-using-eventemitter)
9 changes: 9 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ module.exports = {
'detect-dangerous-redirects': require('./lib/rules/detect-dangerous-redirects'),
'detect-eval-with-expr': require('./lib/rules/detect-eval-with-expr'),
'detect-html-injection': require('./lib/rules/detect-html-injection'),
'detect-improper-exception-handling': require('./lib/rules/detect-improper-exception-handling'),
'detect-insecure-randomness': require('./lib/rules/detect-insecure-randomness'),
'detect-non-literal-require-calls': require('./lib/rules/detect-non-literal-require-calls'),
'detect-nosql-injection': require('./lib/rules/detect-nosql-injection'),
Expand All @@ -23,6 +24,8 @@ module.exports = {
'detect-runinthiscontext-method-in-nodes-vm': require('./lib/rules/detect-runinthiscontext-method-in-nodes-vm'),
'detect-security-missconfiguration-cookie': require('./lib/rules/detect-security-missconfiguration-cookie'),
'detect-sql-injection': require('./lib/rules/detect-sql-injection'),
'detect-unhandled-event-errors': require('./lib/rules/detect-unhandled-event-errors'),
'detect-unhandled-async-errors': require('./lib/rules/detect-unhandled-async-errors'),
'disable-ssl-across-node-server': require('./lib/rules/disable-ssl-across-node-server'),
'non-literal-reg-expr': require('./lib/rules/non-literal-reg-expr')
},
Expand All @@ -34,6 +37,7 @@ module.exports = {
'detect-dangerous-redirects': 0,
'detect-eval-with-expr': 0,
'detect-html-injection': 0,
'detect-improper-exception-handling': 0,
'detect-insecure-randomness': 0,
'detect-non-literal-require-calls': 0,
'detect-nosql-injection': 0,
Expand All @@ -44,6 +48,8 @@ module.exports = {
'detect-runinthiscontext-method-in-nodes-vm': 0,
'detect-security-missconfiguration-cookie': 0,
'detect-sql-injection': 0,
'detect-unhandled-async-errors': 0,
'detect-unhandled-event-errors': 0,
'disable-ssl-across-node-server': 0,
'non-literal-reg-expr': 0
},
Expand All @@ -60,6 +66,7 @@ module.exports = {
'security-node/detect-dangerous-redirects': 'warn',
'security-node/detect-eval-with-expr': 'warn',
'security-node/detect-html-injection': 'warn',
'security-node/detect-improper-exception-handling': 'warn',
'security-node/detect-insecure-randomness': 'warn',
'security-node/detect-non-literal-require-calls': 'warn',
'security-node/detect-nosql-injection': 'warn',
Expand All @@ -70,6 +77,8 @@ module.exports = {
'security-node/detect-runinthiscontext-method-in-nodes-vm': 'warn',
'security-node/detect-security-missconfiguration-cookie': 'warn',
'security-node/detect-sql-injection': 'warn',
'security-node/detect-unhandled-async-errors': 'warn',
'security-node/detect-unhandled-event-errors': 'warn',
'security-node/disable-ssl-across-node-server': 'warn',
'security-node/non-literal-reg-expr': 'warn'
}
Expand Down
105 changes: 105 additions & 0 deletions lib/rules/detect-improper-exception-handling.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* @fileoverview detect improper exception handling
* @author PauMacasaet
*/
'use strict'
const { getDocsUrl } = require('../utils')

function isErrCallbackFunc (calleeArg) {
const functionRegEx = /^(ArrowFunctionExpression|FunctionExpression)$/.test(calleeArg.type)
return functionRegEx
}

module.exports = {
meta: {
type: 'suggestion',
messages: {
msg: 'Detect missing callback on unhandled exception',
msg_no_func_body: 'Detect empty function body',
msg_no_process_exit: 'Detect missing process.exit()',
msg_zero_exit_code: 'Detect missing non-zero exit code'
},
docs: {
description: 'rule that detects improper exception handling',
category: 'Possible Errors',
recommended: true,
url: getDocsUrl('detect-improper-exception-handling')
},
fixable: null
},
create: function (context) {
return {
'CallExpression': function (node) {
let nodeCallee = node.callee
if (nodeCallee.hasOwnProperty('object') &&
nodeCallee.hasOwnProperty('property') &&
nodeCallee.type === 'MemberExpression') {
let objectName = nodeCallee.object.name
let propertyName = nodeCallee.property.name
if (objectName === 'process' && propertyName === 'on') {
if (node.arguments.length > 0) {
if (node.arguments[0].type === 'Literal' &&
node.arguments[0].value === 'uncaughtException')
{
if (node.arguments.length > 1) {
let calleeArgs = node.arguments
let callbacks = []
for (let arg = 1; arg < calleeArgs.length; arg++) {
if (calleeArgs[arg].type === 'CallExpression') {
callbacks.push(calleeArgs[arg].type)
}
if (isErrCallbackFunc(calleeArgs[arg])) {
if (calleeArgs[arg].body.body.length > 0) {
let functionBody = calleeArgs[arg].body.body
for (let i in functionBody) {
if (functionBody[i].type === 'ExpressionStatement') {
if (functionBody[i].expression.callee.hasOwnProperty('object')
&& functionBody[i].expression.callee.hasOwnProperty('property')) {
let objectName = functionBody[i].expression.callee.object.name
let propertyName = functionBody[i].expression.callee.property.name
let hasArguments = functionBody[i].expression.arguments.length > 0
if (objectName === 'process' && propertyName === 'exit' && hasArguments) {
let exitCode = functionBody[i].expression.arguments[0].value
if (exitCode === 1) {
callbacks.push(calleeArgs[arg].type)
}
}
}
}
}
if (callbacks.length === 0) {
context.report({
node: node,
messageId: 'msg_no_process_exit',
loc: {
start: calleeArgs[arg].body.loc.start,
end: calleeArgs[arg].body.loc.end
}
})
}
} else {
context.report({
node: node,
messageId: 'msg_no_func_body',
loc: {
start: calleeArgs[arg].body.loc.start,
end: calleeArgs[arg].body.loc.end
}
})
}
}
}
} else {
context.report({
node: node,
messageId: 'msg'
})
}
}
}
}
}
}
}
}
}
Loading