-
Notifications
You must be signed in to change notification settings - Fork 619
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
make Dest column into clickable links #587
Conversation
admin/ui/route.go
Outdated
@@ -91,7 +91,7 @@ $(function(){ | |||
tbl += '<td>' + (i+1) + '</td>'; | |||
tbl += '<td>' + r.service + '</td>'; | |||
tbl += '<td>' + r.src + '</td>'; | |||
tbl += '<td>' + r.dst + '</td>'; | |||
tbl += '<td><a href="' + r.dst + '">' + r.dst + '</a></td>'; |
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.
Does the attribute have to be escaped? What happens if r.dst contains a quote, for instance?.
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.
I suppose this code is already vulnerable to XSS. I'm gonna fix this in another PR.
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.
I've only seen dst
be an ip:port combo, not sure how clicking on a link has anything to do with XSS?
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.
dst
is an arbitrary string from the configuration and may contain <script>evilCode();</script>
. But again, I've addressed this in #588, so don't worry about it here.
Resolved this and I'll merge this for 1.5.11. Hope I did this correctly. @pschultz could you have a quick look, please? |
@magiconair, yes, looks good to me. |
addresses my issue #508