Skip to content

Commit

Permalink
Merge pull request #50 from NF997/typos
Browse files Browse the repository at this point in the history
Fixed typos and improved grammar
  • Loading branch information
gkouziik authored Aug 30, 2020
2 parents c7222e5 + e168b82 commit 784546a
Show file tree
Hide file tree
Showing 16 changed files with 226 additions and 148 deletions.
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# eslint-plugin-security-node

ESLint plugin for security <br/>
ESLint rules for Node Security <br/>
This plugin will help to identify potential threads,and prevent attacks
ESLint plugin containing Node.js security rules

This plugin will help to identify potential threats and prevent attacks.


## Installation
Expand Down
18 changes: 7 additions & 11 deletions docs/rules/detect-absence-of-name-option-in-exrpress-session.md
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
# Detect the absence of name option in express session (detect-absence-of-name-option-in-exrpress-session)

### Anonymize the sessionID Used
The first step for an attacker targeting a system is reconnaissance. The
attacker researches the environment and narrows possible attack vectors to
optimize the attack. As the defender, we want them to waste as much time
as possible, so keeping the intruder guessing is a good move.
The default implementation of session in express and connect uses connect.sid as
the sessionID token in the cookie. It’s not hard to understand what technolo-
gies are in use based on that. To make it harder for possible attackers to gain
information about the application’s underlying systems, we need to use a more generic name for the session ID:
The first step for an attacker targeting a system is reconnaissance.
The attacker researches the environment and narrows possible attack vectors to optimize the attack.
As the defender, we want them to waste as much time as possible, so keeping the intruder guessing is a good move.
The default implementation of session in express and connect uses connect.sid as the sessionID token in the cookie.
It’s not hard to understand what technologies are in use based on that.
To make it harder for possible attackers to gain information about the application’s underlying systems, we need to use a more generic name for the session ID:

```js
app.use(express.session({
Expand All @@ -18,7 +16,5 @@ resave: false,
saveUninitialized: true
}));
```
With this configuration option, we can stop declaring to the world our session-
handling mechanism and force the attacker to spend more time using various
different attacks that don’t impact our setup.
With this configuration option, we can stop declaring to the world our session-handling mechanism and force the attacker to spend more time using different attacks that don’t impact our setup.

41 changes: 29 additions & 12 deletions docs/rules/detect-buffer-unsafe-allocation.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ Buffer.allocUnsafe(size);
```
**size** (integer) -> The desired length of the new Buffer.

Allocates a new Buffer of size bytes.A zero-length Buffer is created if size is 0.
Allocates a new Buffer of size bytes.
A zero-length Buffer is created if size is 0.
(tip: pay attention to the lines below and the example)

The underlying memory for Buffer instances created in this way is not initialized.The contents of the newly created Buffer are unknown and may contain sensitive data.
The underlying memory for Buffer instances created in this way is not initialized.
The contents of the newly created Buffer are unknown and may contain sensitive data.
Example:

```javascript
Expand All @@ -19,13 +21,19 @@ console.log(buf);
// Prints (contents may vary) CHECK WHAT BUFFER CONTAINTS ->
// <Buffer a0 8b 28 3f 01 00 00 00 50 32>
```
### The question is,where's the problem?
### The question is, where's the problem?
We need to see the whole story here!
Why is buffer class exists in NodeJS API?
Client-side Javscript spares us the need to deal with memory allocation.Like many of its peer languages,in JS the underlying engine (e.g V8) allocates memory and garbage-collect it as needed,making coding simpler and safer.In the browser,preventing access to memory is also necessary to maintain the sandbox JS runs in.
When JS expanded to the server with Node,the browser sandbox was removed,and the need for easy and fast binary data processing increased.To address these needs,Node introduced the Buffer class,which deals with binary data.
Client-side JavaScript spares us the need to deal with memory allocation. Like many of its peer languages, in JS the underlying engine (e.g. V8) allocates memory and garbage-collects it as needed, making coding simpler and safer. In the browser, preventing access to memory is also necessary to maintain the sandbox JS runs in.
When JS expanded to the server with Node, the browser sandbox was removed, and the need for easy and fast binary data processing increased. To address these needs, Node introduced the Buffer class, which deals with binary data.
Client-side JavaScript spares us the need to deal with memory allocation.
Like many of its peer languages, in JS the underlying engine (e.g V8) allocates memory and garbage-collect it as needed, making coding simpler and safer.
In the browser, preventing access to memory is also necessary to maintain the sandbox JS runs in.
When JS expanded to the server with Node, the browser sandbox was removed, and the need for easy and fast binary data processing increased.
To address these needs, Node introduced the Buffer class, which deals with binary data.
Client-side JavaScript spares us the need to deal with memory allocation.
Like many of its peer languages, in JS the underlying engine (e.g. V8) allocates memory and garbage-collects it as needed, making coding simpler and safer.
In the browser, preventing access to memory is also necessary to maintain the sandbox JS runs in.
When JS expanded to the server with Node, the browser sandbox was removed, and the need for easy and fast binary data processing increased.
To address these needs, Node introduced the Buffer class, which deals with binary data.

In versions of Node.js prior to 6.0.0, Buffer instances were created using the Buffer constructor function, which allocates the returned Buffer differently based on what arguments are provided:

Expand All @@ -37,7 +45,12 @@ In versions of Node.js prior to 6.0.0, Buffer instances were created using the B

Because the behavior of new Buffer() is different depending on the type of the first argument, security and reliability issues can be inadvertently introduced into applications when argument validation or Buffer initialization is not performed.

For example, if an attacker can cause an application to receive a number where a string is expected, the application may call new Buffer(100) instead of new Buffer("100"), it will allocate a 100 byte buffer instead of allocating a 3 byte buffer with content "100". This is commonly possible using JSON API calls. Since JSON distinguishes between numeric and string types, it allows injection of numbers where a naive application might expect to always receive a string. Before Node.js 8.0.0, the 100 byte buffer might contain arbitrary pre-existing in-memory data, so may be used to expose in-memory secrets to a remote attacker. Since Node.js 8.0.0, exposure of memory cannot occur because the data is zero-filled. However, other attacks are still possible, such as causing very large buffers to be allocated by the server, leading to performance degradation or crashing on memory exhaustion.
For example, if an attacker can cause an application to receive a number where a string is expected, the application may call new Buffer(100) instead of new Buffer("100"), it will allocate a 100 byte buffer instead of allocating a 3 byte buffer with content "100".
This is commonly possible using JSON API calls.
Since JSON distinguishes between numeric and string types, it allows injection of numbers where a naive application might expect to always receive a string.
Before Node.js 8.0.0, the 100 byte buffer might contain arbitrary pre-existing in-memory data, so may be used to expose in-memory secrets to a remote attacker.
Since Node.js 8.0.0, exposure of memory cannot occur because the data is zero-filled.
However, other attacks are still possible, such as causing very large buffers to be allocated by the server, leading to performance degradation or crashing on memory exhaustion.

To make the creation of Buffer instances more reliable and less error-prone, the various forms of the new Buffer() constructor have been deprecated and replaced by separate Buffer.from(), Buffer.alloc(), and Buffer.allocUnsafe() methods.

Expand All @@ -46,10 +59,14 @@ To make the creation of Buffer instances more reliable and less error-prone, the
* Buffer.allocUnsafe(size) and Buffer.allocUnsafeSlow(size) each return a new uninitialized Buffer of the specified size. Because the Buffer is uninitialized, the allocated segment of memory might contain old data that is potentially sensitive.

### What makes Buffer.allocUnsafe() "unsafe"?
When calling Buffer.allocUnsafe(), the segment of allocated memory is uninitialized (it is not zeroed-out). While this design makes the allocation of memory quite fast, the allocated segment of memory might contain old data that is potentially sensitive. Using a Buffer created by Buffer.allocUnsafe() without completely overwriting the memory can allow this old data to be leaked when the Buffer memory is read

### Where's the problem if i can always use Buffer.alloc() instead of Buffer.allocUnsafe() ?
Allocation is a synchronous operation and we know that single threaded Node.js doesn't really feel good about synchronous stuff. Unsafe allocation is much faster than safe, because the buffer santarization step takes time. Safe allocation is, well, safe, but there is a performance trade off.
When calling Buffer.allocUnsafe(), the segment of allocated memory is uninitialized (it is not zeroed-out).
While this design makes the allocation of memory quite fast, the allocated segment of memory might contain old data that is potentially sensitive.
Using a Buffer created by Buffer.allocUnsafe() without completely overwriting the memory can allow this old data to be leaked when the Buffer memory is read

### Where's the problem if I can always use Buffer.alloc() instead of Buffer.allocUnsafe() ?
Allocation is a synchronous operation and we know that single threaded Node.js doesn't really feel good about synchronous stuff.
Unsafe allocation is much faster than safe because the buffer sanitization step takes time.
Safe allocation is, well, safe, but there is a performance trade-off.


## Further Reading
Expand Down
47 changes: 30 additions & 17 deletions docs/rules/detect-child-process.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ Before we dive into the rule details, we have to clarify **command Injection**

An injection vulnerability manifests when application code sends untrusted user input to an interpreter as part of a command or query!
### What an attacker can do
Using command injection, an attacker can execute arbitary commands on the host operating system of a vulnerable application.This flaw gives enormous opportunities to an attacker,ranging from reading restricted file contents to installing malware with which the attacker can take full control of the server and hot network!
Using command injection, an attacker can execute arbitrary commands on the host operating system of a vulnerable application.
This flaw gives enormous opportunities to an attacker, ranging from reading restricted file contents to installing malware with which the attacker can take full control of the server and hot network!

### Demonstration of an attack
Before we demonstrate how an attacker can cause serious damage to our application lets see how **child_process** works!
The child_process core module enables Node developers to invoke underlying OS commands from the applicaton code.Due to its name and simplicity of use,the child_process.exec method is commonly used for making system calls.
Before we demonstrate how an attacker can cause serious damage to our application, let's see how **child_process** works!
The child_process core module enables Node developers to invoke underlying OS commands from the application code.
Due to its name and simplicity of use,the child_process.exec method is commonly used for making system calls.
The exec method takes three arguments: a command in string format,an optional options object, and a callback function,as demonstrated below
```javascript
child_process.exec(command[,options][, callback])
```
**So where's the problem?** Although the exec method executes OS commands in a nonblocking manner,perfectly aligning with Node's async programming paradigm.its flexibility to pass the command as a string often invites **injection flaws**.This is particularly the case when a **user input** is used to construct the command.
For example,the following example shows using the ``` child_process.exec ``` method to invokethe **gzip** command that appends a user-supplied denamic file path to construct the gzip command.
**So where's the problem?** Although the exec method executes OS commands in a nonblocking manner, perfectly aligning with Node's async programming paradigm, its flexibility to pass the command as a string often invites **injection flaws**.
This is particularly the case when a **user input** is used to construct the command.
For example, the following example shows using the ``` child_process.exec ``` method to invoke the **gzip** command that appends a user-supplied dynamic file path to construct the gzip command.

```javascript
child_process.exec(
Expand All @@ -23,17 +26,21 @@ child_process.exec(
console.log(data);
});
```
To exploit the injection vulnerability in the preceding code,an attacker can append **; rm -rf /** ,for instance, to the file_path input.This allows an attacker to break out of the gzip command context and execute malicious command that delets all files on the server.As part of the user input,an attacker also chain multiple commands by using characters such as **;,&,|,||,$(),<,>,and >>**.
The attack manifests because,under the hood,the exec method spawns a new bin/sh process and passes the command argument for execution to this system shell.This is equivalent to opening a Bash interpreter for an attacker to run any commands with the same privileges as the vulnerable application!
To exploit the injection vulnerability in the preceding code, an attacker can append **; rm -rf /**, for instance, to the file_path input.
This allows an attacker to break out of the gzip command context and execute malicious commands that delete all files on the server.
As part of the user input, an attacker also chain multiple commands by using characters such as **;,&,|,||,$(),<,>, and >>**.
The attack manifests because, under the hood, the exec method spawns a new bin/sh process and passes the command argument for execution to this system shell.
This is equivalent to opening a Bash interpreter for an attacker to run any commands with the same privileges as the vulnerable application!

### Preventing Command Injection
Now that you know the potential of an injection attack to cause severe damage,let's go over methods to prevent it.
Now that you know the potential of an injection attack to cause severe damage, let's go over methods to prevent it.

### Do not use exec with String concatenation
As you saw in the example above,exec spawns a new shell,so if the **command** argument is a user input (string concat may denote user input) ,that may cause serious damage.
As you saw in the example above, exec spawns a new shell, so if the **command** argument is a user input (string concatenation may denote user input), that may cause serious damage.

### Use execFile OR spawn instead of exec
When possible, use the child_process module's **execFile** or **spawn** methods instead of exec.Unlike exec,the spawn and execFile method signatures force developers to separate the command and its arguments.
When possible, use the child_process module's **execFile** or **spawn** methods instead of exec.
Unlike exec, the spawn and execFile method signatures force developers to separate the command and its arguments.
Check the example below,

```javascript
Expand All @@ -47,24 +54,30 @@ child_process.execFile(
console.log(data);
});
```
Any malicious commands chained to file_path user input end up in execFile method's second argument of type array.Any malicious commands in user input are simply ignored or cause a syntax error if they are not relevant to the target command,thus failing the command injection attempts!
Any malicious commands chained to file_path user input end up in execFile method's second argument of type array.
Any malicious commands in user input are simply ignored or cause a syntax error if they are not relevant to the target command, thus failing the command injection attempts!

### Do not use execFile or spawn with option:true
let's dive into **spawn and execFile** methods!
let's dive into **spawn and execFile** methods!
```javscript
child_process.spawn(command[,args][,options])
child_process.execFile(file[,args][,options][,callback])
```

They both have several options that user can manipulate.One of them is the option
**shell:true**. If the shell option is true,then the command runs into a new shell and becomes as dangerous as exec method can be.
They both have several options that a user can manipulate.
One of them is the option **shell:true**.
If the shell option is true, then the command runs into a new shell and becomes as dangerous as exec method can be.

### Input Validation
Although execFile or spawn are safer alternatives to exec,these methods cannot completely prevent command injection.The related scenarios include developers using these methods to invoke a custom script that processes user input,or to execute OS commands(such as find,awk, or sed) that allows passing options to enable file read/write.
Just like other injection attacks,command injection is primarly possible due to indufficient input validation.To protect against it,verify that user-controlled command arguments and command options are valid.
Although execFile or spawn are safer alternatives to exec, these methods cannot completely prevent command injection.
The related scenarios include developers using these methods to invoke a custom script that processes user input or to execute OS commands (such as find, awk, or sed) that allow passing options to enable file read/write.
Just like other injection attacks, command injection is primarily possible due to insufficient input validation.
To protect against it, verify that user-controlled command arguments and command options are valid.

### Using a Whitelist approach for input Validation
When writing the validation logic,use a whitelist approach; that is,define what is permitted and reject any input that doesn't fit this definition.Avoid writing this logic in an opposite manner(blacklist approach), or, in other words,by comparing input against a set of known unsafe characters.Attackers can often find a way to circumvent such filters by being creative with input construction!
When writing the validation logic, use a whitelist approach; that is, define what is permitted and reject any input that doesn't fit this definition.
Avoid writing this logic in an opposite manner(blacklist approach), or, in other words, by comparing input against a set of known unsafe characters.
Attackers can often find a way to circumvent such filters by being creative with input construction!

## Rule Details

Expand Down
Loading

0 comments on commit 784546a

Please sign in to comment.