OWASP/NodeGoat

Broken XSS example

nharraud opened this issue · 0 comments

Hi,

I noticed that commit 7c293e7 has broken the XSS example.

1/ The website property is not saved in the database. Thus it will never be displayed.

profile.updateUser(
parseInt(userId),
firstName,
lastName,
ssn,
dob,
address,
bankAcc,
bankRouting,
(err, user) => {

2/ The website property is not returned after an update
return res.render("profile", {
updateError: "Bank Routing number does not comply with requirements for format specified",
firstNameSafeString,
lastName,
ssn,
dob,
address,
bankAcc,
bankRouting,
environmentalScripts
});

3/ The profile.html page still uses firstNameSafeString as an url, which is confusing.
<a href="{{firstNameSafeString}}">Google search this profile by name</a>

4/ The profile.js:displayProfile does not return firstNameSafeString anymore
doc.website = ESAPI.encoder().encodeForHTML(doc.website)
// fix it by replacing the above with another template variable that is used for
// the context of a URL in a link header
// doc.website = ESAPI.encoder().encodeForURL(doc.website)
return res.render("profile", {
...doc,
environmentalScripts
});

5/ Also shouldn't firstNameSafeString and website be encoded with encodeForHTMLAttribute instead of encodeForHTML and encodeForURL? The current code seems to contradict the tutorial.
// doc.website = ESAPI.encoder().encodeForURL(doc.website)

6/ the firstname is not sanitized after an update.
const firstNameSafeString = firstName