NodeGoat icon indicating copy to clipboard operation
NodeGoat copied to clipboard

Broken XSS example

Open nharraud opened this issue 2 years ago • 0 comments

Hi,

I noticed that commit https://github.com/OWASP/NodeGoat/commit/7c293e721bd1e95be6f82475d295b9b10e3b584e has broken the XSS example.

1/ The website property is not saved in the database. Thus it will never be displayed. https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/routes/profile.js#L82-L91 2/ The website property is not returned after an update https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/routes/profile.js#L65-L75 3/ The profile.html page still uses firstNameSafeString as an url, which is confusing. https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/views/profile.html#L78 4/ The profile.js:displayProfile does not return firstNameSafeString anymore https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/routes/profile.js#L28-L36 5/ Also shouldn't firstNameSafeString and website be encoded with encodeForHTMLAttribute instead of encodeForHTML and encodeForURL? The current code seems to contradict the tutorial. https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/routes/profile.js#L31 6/ the firstname is not sanitized after an update. https://github.com/OWASP/NodeGoat/blob/e2dffdb8c7e988c10bacdccba14d6f0d352c5090/app/routes/profile.js#L64

nharraud avatar Jul 29 '21 13:07 nharraud