Skip to content

Commit

Permalink
Merge pull request #60 from amagitechnologies/master
Browse files Browse the repository at this point in the history
Request to add new rules
  • Loading branch information
gkouziik authored Jan 22, 2022
2 parents e8e3e2b + af9a534 commit 0bb7ff0
Show file tree
Hide file tree
Showing 10 changed files with 752 additions and 0 deletions.
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

0 comments on commit 0bb7ff0

Please sign in to comment.