Skip to content
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

Fixes #2230 #2231

Merged
merged 1 commit into from Apr 22, 2016

Conversation

Projects
None yet
3 participants
@erasche
Copy link
Member

commented Apr 22, 2016

This sanitizes some instances where tool parameters were included directly into the DOM.

workflow/run.mako displayed the parameter as a value in a hidden input. This was base64 encoded as there was no better solution apparent at the time. I'm not sure where this parameter is POSTed to but we should figure that out and b64decode it, or remove the hidden parameter. I logged every use of "tool_state" I could find in lib/ that looked plausible but I couldn't see any uses of it during normal workflow calling.

client/... added the parameter value into the DOM. This was easily sanitized using a standard method.

workflow/display.mako included the parameter value directly into the HTML. This was cgi.esacped

This needs to be backported I believe, I have not tested it yet. Thanks @guerler for being online :)

CC @natefoo @dannon

Fixes #2230
This sanitizes some instances where tool parameters were included
directly into the DOM.

workflow/run.mako displayed the parameter as a value in a hidden input.
This was base64 encoded as there was no better solution apparent at the
time. I'm not sure where this parameter is POSTed to but we should
figure that out and b64decode it, or remove the hidden parameter.

client/... added the parameter value into the DOM. This was easily
sanitized using a standard method.

workflow/display.mako included the parameter value directly into the
HTML. This was cgi.esacped

@erasche erasche added this to the 16.07 milestone Apr 22, 2016

@guerler guerler merged commit 1243bd5 into galaxyproject:dev Apr 22, 2016

3 of 4 checks passed

toolshed test Build finished.
Details
api test Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished.
Details
@guerler

This comment has been minimized.

Copy link
Contributor

commented Apr 22, 2016

Thanks @erasche

@erasche

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

@natefoo did we ever end up backporting this? You mentioned you were working on it in IRC

@martenson

This comment has been minimized.

Copy link
Member

commented May 2, 2016

@erasche this is not in 16.04 yet; @natefoo has more pressing agenda now (jetstream), is there anything difficult here or can I just backport it?

@erasche

This comment has been minimized.

Copy link
Member Author

commented May 2, 2016

TIL Jetstream > XSS. @martenson this is a stored XSS which applies to every version of Galaxy. It should be easy enough to backport, please do so!

@martenson martenson referenced this pull request May 2, 2016

Merged

[16.04] Backport of 2230 #2284

@erasche erasche deleted the erasche:fix-2230 branch Dec 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.