From f62ebd16770ea95618862442c2e84e8d67a76d20 Mon Sep 17 00:00:00 2001 From: sns Date: Mon, 6 Aug 2018 11:44:11 +0530 Subject: [PATCH] Added structure for 2017 --- docs/SUMMARY.md | 18 +++++--- docs/solution/a1-injection.md | 6 +-- docs/solution/a10-insufficient-logging.md | 38 +++++++++++++++ docs/solution/a2-broken-auth.md | 5 +- ...osure.md => a3-sensitive-data-exposure.md} | 4 +- docs/solution/a4-idor.md | 46 ------------------- docs/solution/a4-xxe.md | 42 +++++++++++++++++ ...control.md => a5-broken-access-control.md} | 44 +++++++++++++++++- ...y-misconfig.md => a6-securty-misconfig.md} | 6 +-- docs/solution/{a3-xss.md => a7-xss.md} | 5 +- docs/solution/a8-insecure-deserialization.md | 42 +++++++++++++++++ ...ing-components-with-known-vulnerability.md | 4 +- docs/solution/{a8-csrf.md => ax-csrf.md} | 0 ... ax-unvalidated-redirects-and-forwards.md} | 0 14 files changed, 192 insertions(+), 68 deletions(-) create mode 100644 docs/solution/a10-insufficient-logging.md rename docs/solution/{a6-sensitive-data-exposure.md => a3-sensitive-data-exposure.md} (89%) delete mode 100644 docs/solution/a4-idor.md create mode 100644 docs/solution/a4-xxe.md rename docs/solution/{a7-missing-function-level-access-control.md => a5-broken-access-control.md} (58%) rename docs/solution/{a5-securty-misconfig.md => a6-securty-misconfig.md} (87%) rename docs/solution/{a3-xss.md => a7-xss.md} (93%) create mode 100644 docs/solution/a8-insecure-deserialization.md rename docs/solution/{a8-csrf.md => ax-csrf.md} (100%) rename docs/solution/{a10-unvalidated-redirects-and-forwards.md => ax-unvalidated-redirects-and-forwards.md} (100%) diff --git a/docs/SUMMARY.md b/docs/SUMMARY.md index acbf670..66174ac 100644 --- a/docs/SUMMARY.md +++ b/docs/SUMMARY.md @@ -11,14 +11,18 @@ * [A1 Injection](solution/a1-injection.md) * [A2 Broken Authentication and Session Management](solution/a2-broken-auth.md) -* [A3 Cross-site Scripting](solution/a3-xss.md) -* [A4 Insecure Direct Object Reference](solution/a4-idor.md) -* [A5 Security Misconfiguration](solution/a5-securty-misconfig.md) -* [A6 Sensitive Data Exposure](solution/a6-sensitive-data-exposure.md) -* [A7 Missing Function Level Access Control](solution/a7-missing-function-level-access-control.md) -* [A8 Cross-site Request Forgery](solution/a8-csrf.md) +* [A3 Sensitive Data Exposure](solution/a3-sensitive-data-exposure.md) +* [A4 XML External Entities](solution/a4-xxe.md) +* [A5 Broken Access Control](solution/a5-broken-access-control.md) +* [A6 Security Misconfiguration](solution/a6-securty-misconfig.md) +* [A7 Cross-site Scripting](solution/a7-xss.md) +* [A8 Insecure Deseriliaztion](solution/a8-insecure-deserialization.md) * [A9 Using Components with Known Vulnerability](solution/a9-using-components-with-known-vulnerability.md) -* [A10 Unvalidated Redirects and Forwards](solution/a10-unvalidated-redirects-and-forwards.md) +* [A10 Insufficient Logging and Monitoring](solution/a10-insufficient-logging.md) + +## Top 10 2013 +* [A8:2013 Cross-site Request Forgery](solution/ax-csrf.md) +* [A10:2013 Unvalidated Redirects and Forwards](solution/ax-unvalidated-redirects-and-forwards.md) ## About diff --git a/docs/solution/a1-injection.md b/docs/solution/a1-injection.md index 88305b8..c671f7d 100644 --- a/docs/solution/a1-injection.md +++ b/docs/solution/a1-injection.md @@ -98,6 +98,6 @@ Implemented in the following files **Reference** -- https://www.owasp.org/index.php/Top_10_2013-A1-Injection -- https://snyk.io/blog/sql-injection-orm-vulnerabilities/ -- https://hackernoon.com/nodejs-security-issue-javascript-node-example-tutorial-vulnerabilities-hack-line-url-command-injection-412011924d1b \ No newline at end of file +- +- +- \ No newline at end of file diff --git a/docs/solution/a10-insufficient-logging.md b/docs/solution/a10-insufficient-logging.md new file mode 100644 index 0000000..e0d975d --- /dev/null +++ b/docs/solution/a10-insufficient-logging.md @@ -0,0 +1,38 @@ +# Insufficient Logging and Monitoring + +Logging. Logging. + +**Vulnerable Code snippet** + +*core/appHandler.js* +``` +... + +... +``` +**Solution** + + +*core/appHandler.js* +``` +... + +... +``` + +But it is recommended to explicitly validate/sanitize inputs + +**Fixes** + +Implemented in the following files + +- *core/appHandler.js* + +**Recommendation** + +- Validate Input before processing +- Sanitize Input before storing + +**Reference** + +- \ No newline at end of file diff --git a/docs/solution/a2-broken-auth.md b/docs/solution/a2-broken-auth.md index 10c4829..a56c0bc 100644 --- a/docs/solution/a2-broken-auth.md +++ b/docs/solution/a2-broken-auth.md @@ -68,5 +68,6 @@ Implemented in the following files **References** -- https://www.owasp.org/index.php/Broken_Authentication_and_Session_Management -- https://www.owasp.org/index.php/Forgot_Password_Cheat_Sheet \ No newline at end of file +- +- +- \ No newline at end of file diff --git a/docs/solution/a6-sensitive-data-exposure.md b/docs/solution/a3-sensitive-data-exposure.md similarity index 89% rename from docs/solution/a6-sensitive-data-exposure.md rename to docs/solution/a3-sensitive-data-exposure.md index aa0b2ae..8058b60 100644 --- a/docs/solution/a6-sensitive-data-exposure.md +++ b/docs/solution/a3-sensitive-data-exposure.md @@ -70,5 +70,5 @@ Implemented in the following files **Reference** -- https://www.owasp.org/index.php/Top_10_2013-A6-Sensitive_Data_Exposure -- https://stackoverflow.com/questions/28927836/prevent-sequelize-from-outputting-sql-to-the-console-on-execution-of-query \ No newline at end of file +- +- \ No newline at end of file diff --git a/docs/solution/a4-idor.md b/docs/solution/a4-idor.md deleted file mode 100644 index 455ce93..0000000 --- a/docs/solution/a4-idor.md +++ /dev/null @@ -1,46 +0,0 @@ -# Insecure Direct Object Reference - -An Insecure Direct Object Reference \(IDOR\) vulnerability exists in `Edit User` functionality. Particularly `userEditSubmit` method fails to validate `id` parameter to ensure that the calling user has appropriate access to the object. This issue can be exploited to reset information for any user identified by id. - -http://127.0.0.1:9090/app/useredit - -**Vulnerable Code snippet** - -*core/apphandler.js* -``` -... -module.exports.userEditSubmit = function(req,res){ - if(req.body.password==req.body.cpassword){ - db.User.find({where:{'id':req.body.id}}).then(user=>{ - if(user){ - user.password = bCrypt.hashSync(req.body.password, bCrypt.genSaltSync(10), null) - user.save().then(function(){ -... -``` - -Simply changing the user id in the page can lead to exploitation.

-![idor1](/resources/idor1.png "IDOR") - -**Solution** - -A simple check can solve this issue -``` -if (req.user.id == req.body.id) - //do -else - //dont -``` - -In our case we can use passports user object at `req.user` for modifying user information - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -**Recommendataion** -Consider any user supplied information as untrusted and always validate user access by sessions. - -**Reference** -- https://www.owasp.org/index.php/Top_10_2013-A4-Insecure_Direct_Object_References \ No newline at end of file diff --git a/docs/solution/a4-xxe.md b/docs/solution/a4-xxe.md new file mode 100644 index 0000000..f881edf --- /dev/null +++ b/docs/solution/a4-xxe.md @@ -0,0 +1,42 @@ +# XML External Entities + +## XXE in XYZ + +There is a SQL Injection in `User Search` feature at the following URL + +http://127.0.0.1:9090/app/usersearch + +**Vulnerable Code snippet** + +*core/appHandler.js* +``` +... + +... +``` +**Solution** + + +*core/appHandler.js* +``` +... + +... +``` + +But it is recommended to explicitly validate/sanitize inputs + +**Fixes** + +Implemented in the following files + +- *core/appHandler.js* + +**Recommendation** + +- Validate Input before processing +- Sanitize Input before storing + +**Reference** + +- \ No newline at end of file diff --git a/docs/solution/a7-missing-function-level-access-control.md b/docs/solution/a5-broken-access-control.md similarity index 58% rename from docs/solution/a7-missing-function-level-access-control.md rename to docs/solution/a5-broken-access-control.md index 7a1ece2..e940262 100644 --- a/docs/solution/a7-missing-function-level-access-control.md +++ b/docs/solution/a5-broken-access-control.md @@ -61,10 +61,52 @@ Implemented in the following files - *routes/app.js* - *views/app/admin.ejs* +## Missing Authorization check in Edit User + +The `userEditSubmit` method fails to validate `id` parameter to ensure that the calling user has appropriate access to the object. This issue can be exploited to reset information for any user identified by id. + +http://127.0.0.1:9090/app/useredit + +**Vulnerable Code snippet** + +*core/apphandler.js* +``` +... +module.exports.userEditSubmit = function(req,res){ + if(req.body.password==req.body.cpassword){ + db.User.find({where:{'id':req.body.id}}).then(user=>{ + if(user){ + user.password = bCrypt.hashSync(req.body.password, bCrypt.genSaltSync(10), null) + user.save().then(function(){ +... +``` + +Simply changing the user id in the page can lead to exploitation.

+![idor1](/resources/idor1.png "IDOR") + +**Solution** + +A simple check can solve this issue +``` +if (req.user.id == req.body.id) + //do +else + //dont +``` + +In our case we can use passports user object at `req.user` for modifying user information + +**Fixes** + +Implemented in the following files + +- *core/appHandler.js* + **Recommendation** - Try to restrict your functions to maximum extent, White listing is always better than blacklisting +- Consider any user supplied information as untrusted and always validate user access by sessions **Reference** -- https://www.owasp.org/index.php/Top_10_2013-A7-Missing_Function_Level_Access_Control +- diff --git a/docs/solution/a5-securty-misconfig.md b/docs/solution/a6-securty-misconfig.md similarity index 87% rename from docs/solution/a5-securty-misconfig.md rename to docs/solution/a6-securty-misconfig.md index 178ba3d..d03d03c 100644 --- a/docs/solution/a5-securty-misconfig.md +++ b/docs/solution/a6-securty-misconfig.md @@ -56,6 +56,6 @@ Implemented in the following files - *server.js* **Reference** -- https://expressjs.com/en/advanced/best-practice-security.html -- https://www.owasp.org/index.php/Top_10_2013-A5-Security_Misconfiguration -- https://blog.risingstack.com/node-js-security-checklist/ \ No newline at end of file +- +- +- \ No newline at end of file diff --git a/docs/solution/a3-xss.md b/docs/solution/a7-xss.md similarity index 93% rename from docs/solution/a3-xss.md rename to docs/solution/a7-xss.md index a6be97d..0f48270 100644 --- a/docs/solution/a3-xss.md +++ b/docs/solution/a7-xss.md @@ -87,5 +87,6 @@ Implemented in the following files **Reference** -- https://www.owasp.org/index.php/Cross-site_Scripting_(XSS) -- https://www.npmjs.com/package/xss-filters +- +- +- diff --git a/docs/solution/a8-insecure-deserialization.md b/docs/solution/a8-insecure-deserialization.md new file mode 100644 index 0000000..592e5de --- /dev/null +++ b/docs/solution/a8-insecure-deserialization.md @@ -0,0 +1,42 @@ +# Insecure Deserialization + +## Insecure Deserialization in XYZ + +There is a SQL Injection in `User Search` feature at the following URL + +http://127.0.0.1:9090/app/usersearch + +**Vulnerable Code snippet** + +*core/appHandler.js* +``` +... + +... +``` +**Solution** + + +*core/appHandler.js* +``` +... + +... +``` + +But it is recommended to explicitly validate/sanitize inputs + +**Fixes** + +Implemented in the following files + +- *core/appHandler.js* + +**Recommendation** + +- Validate Input before processing +- Sanitize Input before storing + +**Reference** + +- \ No newline at end of file diff --git a/docs/solution/a9-using-components-with-known-vulnerability.md b/docs/solution/a9-using-components-with-known-vulnerability.md index 62a4886..6745ef1 100644 --- a/docs/solution/a9-using-components-with-known-vulnerability.md +++ b/docs/solution/a9-using-components-with-known-vulnerability.md @@ -46,5 +46,5 @@ Implemented in the following files **Reference** -- https://www.owasp.org/index.php/Top_10_2013-A9-Using_Components_with_Known_Vulnerabilities -- https://blog.travis-ci.com/2017-04-20-continuous-security-snyk-travis-ci/ +- +- diff --git a/docs/solution/a8-csrf.md b/docs/solution/ax-csrf.md similarity index 100% rename from docs/solution/a8-csrf.md rename to docs/solution/ax-csrf.md diff --git a/docs/solution/a10-unvalidated-redirects-and-forwards.md b/docs/solution/ax-unvalidated-redirects-and-forwards.md similarity index 100% rename from docs/solution/a10-unvalidated-redirects-and-forwards.md rename to docs/solution/ax-unvalidated-redirects-and-forwards.md