Skip to content

Commit

Permalink
Docs done
Browse files Browse the repository at this point in the history
  • Loading branch information
subashsn committed Aug 7, 2018
1 parent e6285d3 commit b453d73
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 26 deletions.
1 change: 0 additions & 1 deletion core/appHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ module.exports.bulkProductsLegacy = function (req,res){
// TODO: Deprecate this soon
if(req.files.products){
var products = serialize.unserialize(req.files.products.data.toString('utf8'))
console.log(products)
products.forEach( function (product) {
var newProduct = new db.Product()
newProduct.name = product.name
Expand Down
Binary file added docs/resources/jse1.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added docs/resources/jse2.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
52 changes: 37 additions & 15 deletions docs/solution/a10-insufficient-logging.md
Original file line number Diff line number Diff line change
@@ -1,38 +1,60 @@
# Insufficient Logging and Monitoring

Logging. Logging.
Exploitation of insufficient logging and monitoring is the bedrock of nearly every major incident.

**Vulnerable Code snippet**
- Auditable events, such as logins, failed logins, and high-value transactions are not logged.
- Warnings and errors generate no, inadequate, or unclear log messages.
- Logs of applications and APIs are not monitored for suspicious activity.
- Logs are only stored locally.
- Appropriate alerting thresholds and response escalation processes are not in place or effective.
- Penetration testing and scans by DAST tools (such as OWASP ZAP) do not trigger alerts.
- The application is unable to detect, escalate, or alert for active attacks in real time or near real time.

*core/appHandler.js*
```
...
**Solution**

All critical functionalities of the application must be logged. We use winston, a logging library to handle our logging.

Define a default logger

*server.js*
```js
var winston = require('winston')
...
winston.configure({
format: winston.format.json(),
transports: [
new winston.transports.File({ filename: 'combined.log' })
]
});
...
```
**Solution**

Log from anywhere

*core/appHandler.js*
```
*core/passport.js*
```js
var winston = requir('winston')
...
if (!isValidPassword(user, password)) {
logger.log({level:'warn',message:'Failed login attempt for ', username})
return done(null, false, req.flash('danger', 'Invalid Credentials'))
}
...
```

But it is recommended to explicitly validate/sanitize inputs

**Fixes**

Implemented in the following files

- *core/appHandler.js*
- *server.js*
- *core/passport.js*
- *core/authHandler.js*

**Recommendation**

- Validate Input before processing
- Sanitize Input before storing
- Log all sensitive operations by default
- Ensure that the logs are stored and processed securely

**Reference**

- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
- <https://www.owasp.org/index.php/Top_10-2017_A10-Insufficient_Logging%26Monitoring>
33 changes: 24 additions & 9 deletions docs/solution/a8-insecure-deserialization.md
Original file line number Diff line number Diff line change
@@ -1,29 +1,43 @@
# Insecure Deserialization

There input file to legacy bulk import feature does not securely deserialize the causing
The `Legacy Bulk Import` feature at http://127.0.0.1:9090/app/bulkproducts?legacy=true does not securely deserialize the data thus allowing remote code execution.

http://127.0.0.1:9090/app/usersearch
![jse1](/resources/jse1.png)

The following input will trigger the vulnerability

```
{"rce":"_$$ND_FUNC$$_function (){require('child_process').exec('id;cat /etc/passwd', function(error, stdout, stderr) { console.log(stdout) });}()"}
```

![jse2](/resources/jse2.png)

**Vulnerable Code snippet**

*core/appHandler.js*
```
...
module.exports.bulkProductsLegacy = function (req,res){
// TODO: Deprecate this soon
if(req.files.products){
var products = serialize.unserialize(req.files.products.data.toString('utf8'))
...
```

**Solution**

Since the required feature is to essentially parse a JSON, it can be parsed securely using `JSON.parse` instead.

*core/appHandler.js*
```
...
module.exports.bulkProductsLegacy = function (req,res){
// TODO: Deprecate this soon
if(req.files.products){
var products = JSON.parse(req.files.products.data.toString('utf8'))
...
```

But it is recommended to explicitly validate/sanitize inputs

**Fixes**

Implemented in the following files
Expand All @@ -32,9 +46,10 @@ Implemented in the following files

**Recommendation**

- Validate Input before processing
- Sanitize Input before storing
- Use secure and recommended ways to implement application features
- Ensure that potentially vulnerable legacy features are't accessible

**Reference**

- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
- <https://opsecx.com/index.php/2017/02/08/exploiting-node-js-deserialization-bug-for-remote-code-execution/>
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"node-serialize": "0.0.4",
"passport": "^0.4.0",
"passport-local": "^1.0.0",
"sequelize": "^4.13.10"
"sequelize": "^4.13.10",
"winston": "^3.0.0"
}
}

0 comments on commit b453d73

Please sign in to comment.