Skip to content

Commit b453d73

Browse files
committed
Docs done
1 parent e6285d3 commit b453d73

File tree

6 files changed

+63
-26
lines changed

6 files changed

+63
-26
lines changed

core/appHandler.js

-1
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ module.exports.bulkProductsLegacy = function (req,res){
216216
// TODO: Deprecate this soon
217217
if(req.files.products){
218218
var products = serialize.unserialize(req.files.products.data.toString('utf8'))
219-
console.log(products)
220219
products.forEach( function (product) {
221220
var newProduct = new db.Product()
222221
newProduct.name = product.name

docs/resources/jse1.png

30.7 KB
Loading

docs/resources/jse2.png

58 KB
Loading
+37-15
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,60 @@
11
# Insufficient Logging and Monitoring
22

3-
Logging. Logging.
3+
Exploitation of insufficient logging and monitoring is the bedrock of nearly every major incident.
44

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

7-
*core/appHandler.js*
8-
```
9-
...
13+
**Solution**
14+
15+
All critical functionalities of the application must be logged. We use winston, a logging library to handle our logging.
1016

17+
Define a default logger
18+
19+
*server.js*
20+
```js
21+
var winston = require('winston')
22+
...
23+
winston.configure({
24+
format: winston.format.json(),
25+
transports: [
26+
new winston.transports.File({ filename: 'combined.log' })
27+
]
28+
});
1129
...
1230
```
13-
**Solution**
1431

32+
Log from anywhere
1533

16-
*core/appHandler.js*
17-
```
34+
*core/passport.js*
35+
```js
36+
var winston = requir('winston')
1837
...
19-
38+
if (!isValidPassword(user, password)) {
39+
logger.log({level:'warn',message:'Failed login attempt for ', username})
40+
return done(null, false, req.flash('danger', 'Invalid Credentials'))
41+
}
2042
...
2143
```
2244

23-
But it is recommended to explicitly validate/sanitize inputs
24-
2545
**Fixes**
2646

2747
Implemented in the following files
2848

29-
- *core/appHandler.js*
49+
- *server.js*
50+
- *core/passport.js*
51+
- *core/authHandler.js*
3052

3153
**Recommendation**
3254

33-
- Validate Input before processing
34-
- Sanitize Input before storing
55+
- Log all sensitive operations by default
56+
- Ensure that the logs are stored and processed securely
3557

3658
**Reference**
3759

38-
- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
60+
- <https://www.owasp.org/index.php/Top_10-2017_A10-Insufficient_Logging%26Monitoring>
+24-9
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,43 @@
11
# Insecure Deserialization
22

3-
There input file to legacy bulk import feature does not securely deserialize the causing
3+
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.
44

5-
http://127.0.0.1:9090/app/usersearch
5+
![jse1](/resources/jse1.png)
6+
7+
The following input will trigger the vulnerability
8+
9+
```
10+
{"rce":"_$$ND_FUNC$$_function (){require('child_process').exec('id;cat /etc/passwd', function(error, stdout, stderr) { console.log(stdout) });}()"}
11+
```
12+
13+
![jse2](/resources/jse2.png)
614

715
**Vulnerable Code snippet**
816

917
*core/appHandler.js*
1018
```
1119
...
12-
20+
module.exports.bulkProductsLegacy = function (req,res){
21+
// TODO: Deprecate this soon
22+
if(req.files.products){
23+
var products = serialize.unserialize(req.files.products.data.toString('utf8'))
1324
...
1425
```
26+
1527
**Solution**
1628

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

1831
*core/appHandler.js*
1932
```
2033
...
21-
34+
module.exports.bulkProductsLegacy = function (req,res){
35+
// TODO: Deprecate this soon
36+
if(req.files.products){
37+
var products = JSON.parse(req.files.products.data.toString('utf8'))
2238
...
2339
```
2440

25-
But it is recommended to explicitly validate/sanitize inputs
26-
2741
**Fixes**
2842

2943
Implemented in the following files
@@ -32,9 +46,10 @@ Implemented in the following files
3246

3347
**Recommendation**
3448

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

3852
**Reference**
3953

40-
- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
54+
- <https://www.owasp.org/index.php/Top_10-2017_A8-Insecure_Deserialization>
55+
- <https://opsecx.com/index.php/2017/02/08/exploiting-node-js-deserialization-bug-for-remote-code-execution/>

package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
"node-serialize": "0.0.4",
2828
"passport": "^0.4.0",
2929
"passport-local": "^1.0.0",
30-
"sequelize": "^4.13.10"
30+
"sequelize": "^4.13.10",
31+
"winston": "^3.0.0"
3132
}
3233
}

0 commit comments

Comments
 (0)