-
Couldn't load subscription status.
- Fork 2k
Description
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.
NodeGoat/app/routes/profile.js
Lines 82 to 91 in e2dffdb
| profile.updateUser( | |
| parseInt(userId), | |
| firstName, | |
| lastName, | |
| ssn, | |
| dob, | |
| address, | |
| bankAcc, | |
| bankRouting, | |
| (err, user) => { |
2/ The
website property is not returned after an updateNodeGoat/app/routes/profile.js
Lines 65 to 75 in e2dffdb
| 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. NodeGoat/app/views/profile.html
Line 78 in e2dffdb
| <a href="{{firstNameSafeString}}">Google search this profile by name</a> |
4/ The
profile.js:displayProfile does not return firstNameSafeString anymoreNodeGoat/app/routes/profile.js
Lines 28 to 36 in e2dffdb
| 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.NodeGoat/app/routes/profile.js
Line 31 in e2dffdb
| // doc.website = ESAPI.encoder().encodeForURL(doc.website) |
6/ the
firstname is not sanitized after an update.NodeGoat/app/routes/profile.js
Line 64 in e2dffdb
| const firstNameSafeString = firstName |