Fix(User): Fix XSS vulnerability for revoke token endpoint #2751
Conversation
hoangnt2
requested review from
heliocastro,
GMishx,
KoukiHama,
arunazhakesan and
ag4ums
as code owners
Signed-off-by: hoangnt2 <[email protected]>
hoangnt2
force-pushed
the
release/fix-xss-error
branch
from
35c87f8 to
6054165
Compare
There was a problem hiding this comment.
@hoangnt2 I started a branch deprecating commons-lang to commons-lang3.
This method you used moved to commons-text
| import org.apache.commons.lang.StringEscapeUtils; | ||
| import org.apache.thrift.TException; |
There was a problem hiding this comment.
Change for org.apache.commons.text.StringEscapeUtils
| @@ -368,7 +367,7 @@ public ResponseEntity<String> revokeUserRestApiToken( | |||
| User sw360User = restControllerHelper.getSw360UserFromAuthentication(); | |||
|
|
|||
| if (!userService.isTokenNameExisted(sw360User, tokenName)) { | |||
| return new ResponseEntity<>("Token not found: " + StringEscapeUtils.escapeHtml(tokenName), HttpStatus.NOT_FOUND); | |||
| return new ResponseEntity<>("Token not found", HttpStatus.NOT_FOUND); | |||
There was a problem hiding this comment.
Change for escapeHtml4
@heliocastro Escape HTML cannot resolve XSS problem, CodeQL still warns so I decided to remove tokenName from response. I also want to know, is there a way to run this check locally. I want to know if my fix can solve the problem before creating a PR |
Ok, for that part is fine, agreed. But remove the function unfortunately solve a small portion. We have this pervasive on the code. |
|
@heliocastro OK, I will try again with escapeHtml4. Hopefully, it can solve the problem. |
|
Since #2750 is merged, this PR can be updated as well. |
Fix XSS vulnerability for revoke token endpoint
CodeQL warning: https://github.com/eclipse-sw360/sw360/security/code-scanning/87