-
Notifications
You must be signed in to change notification settings - Fork 5.9k
fix: escape error.message on login failure #3695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3695 +/- ##
==========================================
+ Coverage 60.22% 61.55% +1.33%
==========================================
Files 35 35
Lines 1810 1813 +3
Branches 365 365
==========================================
+ Hits 1090 1116 +26
+ Misses 604 588 -16
+ Partials 116 109 -7
Continue to review full report at Codecov.
|
e5c705c
to
2e76ae1
Compare
2e76ae1
to
beaf3ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one small nit; though I'll leave it to you on if you want it to stay as-is or update the code!
157abcf
to
f7e445d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒
**/ | ||
export function escapeHtml(unsafe: string): string { | ||
return unsafe | ||
.replace(/&/g, "&") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems kinda unfortunate that there isn't a built-in function for doing this? I'm always wary of escaping things incorrectly, but this seems reasonable enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it is built-in but:
Although
escape()
is not strictly deprecated (as in "removed from the Web standards"), it is defined in Annex B of the ECMA-262 standard, whose introduction states:
Programmers should not use or assume the existence of these features and behaviors when writing new ECMAScript code
This can be used to escape any special characters in a string with HTML before sending from the server back to the client. This is important to prevent a cross-site scripting attack.
f7e445d
to
c0e123a
Compare
This PR fixes a security issue by sanitizing the
error.message
before sending it back to the client on a login failure.Changes
escapeHtml
+ testserror.message
before sending to client on login failureChecklist
Fixes #3382