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.

Sign up for GitHub

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

Jump to bottom

review: security patch to prevent code injection #2945

Merged
merged 2 commits into from
Feb 15, 2019
Merged

review: security patch to prevent code injection #2945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe57c38
Autoescape input markup to remove code injection vulnerability
sadielbartholomew Feb 6, 2019
1ba9521
Respond to feedback: fix urlise
sadielbartholomew Feb 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 17 additions & 15 deletions lib/cylc/cylc-review/template/cycles.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
<div class="input-group">
<label for="order">Sort Order</label>
<select name="order" title="Sort Order" class="form-control">
{% for k, v in [("time_desc", "new-&gt;old"),
("time_asc", "old-&gt;new")] -%}
{% for k, v in [("time_desc", "new-&gt;old" | safe ),
("time_asc", "old-&gt;new" | safe )] -%}
<option
{% if order and order == k -%}selected="selected"{% endif -%}
value="{{k}}">{{v}}</option>
Expand Down Expand Up @@ -140,8 +140,8 @@
{% for entry in entries -%}
{% set cycle_in_url = entry.cycle|replace('+', '%2B') -%}
{% set task_jobs_url = (
script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~
suite|replace('+', '%2F') ~ "&amp;cycles=" ~ cycle_in_url
(script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~
suite|replace('+', '%2F') ~ "&amp;cycles=" ~ cycle_in_url) | safe
) -%}
<tr class="entry">

Expand All @@ -162,21 +162,22 @@
{# task and jobs states -#}
{% for state, icon, label, title_, unit1, unit2, url_arg in [
("active", "play", "info", "active", "task", "tasks",
"&amp;task_status=" ~
task_status_groups["active"]|join("&amp;task_status=")),
("&amp;task_status=" ~
task_status_groups["active"]|join("&amp;task_status=")) | safe ),
("job_active", "play-circle", "info", "active", "job", "jobs",
"&amp;job_status=submitted,running"),
"&amp;job_status=submitted,running" | safe ),
("success", "ok", "success", "succeeded", "task", "tasks",
"&amp;task_status=" ~
task_status_groups["success"]|join("&amp;task_status=")),
("&amp;task_status=" ~
task_status_groups["success"]|join("&amp;task_status=")) | safe ),
("job_success", "ok-circle", "success", "succeeded", "job", "jobs",
"&amp;job_status=succeeded"),
"&amp;job_status=succeeded" | safe ),
("fail", "remove", "danger", "failed", "task", "tasks",
"&amp;task_status=" ~
task_status_groups["fail"]|join("&amp;task_status=")),
("&amp;task_status=" ~
task_status_groups["fail"]|join("&amp;task_status=")) | safe ),
("job_fail", "remove-circle", "danger", "failed", "job", "jobs",
"&amp;job_status=submission-failed,failed"),
"&amp;job_status=submission-failed,failed" | safe ),
] -%}

{% set n_state = entry.n_states[state] -%}
{% set unit = unit1 -%}
{% if n_state -%}
Expand Down Expand Up @@ -206,8 +207,9 @@
<td>
{% if entry.has_log_job_tar_gz -%}
{% set download = (
script ~ "/view/" ~ user ~ "?&amp;suite=" ~ suite|replace('+', '%2F') ~
"&amp;path=log/job-" ~ cycle_in_url ~ ".tar.gz&amp;mode=download"
(script ~ "/view/" ~ user ~ "?&amp;suite=" ~
suite|replace('+', '%2F') ~ "&amp;path=log/job-" ~ cycle_in_url ~
".tar.gz&amp;mode=download") | safe
) -%}
<a href="{{download}}"
download="{{user}}-{{suite|replace("/", "%2F")}}-log-job-{{entry.cycle}}.tar.gz">
Expand Down
24 changes: 13 additions & 11 deletions lib/cylc/cylc-review/template/job-entry.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
{% endif -%}
{% set cycle_str = entry.cycle|replace('+', '%2B') -%}
{% set taskjobs_link = (
script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~ suite|replace('+', '%2F') ~ no_fuzzy_time_str
(script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~
suite|replace('+', '%2F') ~ no_fuzzy_time_str) | safe
) -%}
{% set view_link = (
script ~ "/view/" ~ user ~ "?&amp;suite=" ~ suite|replace('+', '%2F') ~ no_fuzzy_time_str
(script ~ "/view/" ~ user ~ "?&amp;suite=" ~ suite|replace('+', '%2F') ~
no_fuzzy_time_str) | safe
) -%}
<tr class="entry"><!-- entry row -->
<td>
Expand All @@ -17,22 +19,22 @@
{% set icon = "ok" %}
{% set label_class = "label-success" %}
{% set url_arg = (
"&amp;task_status=" ~
task_status_groups["success"]|join("&amp;task_status=")
("&amp;task_status=" ~
task_status_groups["success"]|join("&amp;task_status=")) | safe
) -%}
{% elif entry.task_status in ["failed", "submission failed"] -%}
{% set icon = "remove" %}
{% set label_class = "label-danger" %}
{% set url_arg = (
"&amp;task_status=" ~
task_status_groups["fail"]|join("&amp;task_status=")
("&amp;task_status=" ~
task_status_groups["fail"]|join("&amp;task_status=")) | safe
) -%}
{% else -%}
{% set icon = "play" %}
{% set label_class = "label-info" %}
{% set url_arg = (
"&amp;task_status=" ~
task_status_groups["active"]|join("&amp;task_status=")
("&amp;task_status=" ~
task_status_groups["active"]|join("&amp;task_status=")) | safe
) -%}
{% endif -%}
<small>
Expand All @@ -47,8 +49,8 @@
<!-- entry: submit_status, run_status -->
<small>
{% set link = (
script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~ suite|replace('+', '%2F') ~
no_fuzzy_time_str
(script ~ "/taskjobs/" ~ user ~ "?&amp;suite=" ~
suite|replace('+', '%2F') ~ no_fuzzy_time_str) | safe
) -%}
{% if entry.run_status == 0 -%}
<a href="{{taskjobs_link}}&amp;job_status=succeeded"
Expand Down Expand Up @@ -161,7 +163,7 @@
<ul class="list-inline">
{% for key, log in entry.logs|dictsort if not log.seq_key -%}
{% if key in ["job.out", "job.err"] -%}
{% set key_str = "<strong>" ~ key ~ "</strong>" -%}
{% set key_str = ("<strong>" ~ key ~ "</strong>") | safe -%}
{% else -%}
{% set key_str = key -%}
{% endif -%}
Expand Down
4 changes: 2 additions & 2 deletions lib/cylc/cylc-review/template/suites.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@
<label for="order">Sort Order</label>
<select name="order" title="Sort Order" class="form-control">
{% for k, v in [
("time_desc", "new-&gt;old"),
("time_asc", "old-&gt;new"),
("time_desc", "new-&gt;old" | safe ),
("time_asc", "old-&gt;new" | safe ),
("name_asc", "a-z suite name"),
("name_desc", "z-a suite name"),
] -%}
Expand Down
48 changes: 24 additions & 24 deletions lib/cylc/cylc-review/template/taskjobs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
<fieldset class="container-fluid">
<div class="row">
{% for key, name, value in [
("cycles",
"Cycles (before, after or patterns): &lt;CYCLE | &gt;CYCLE | GLOB ...", cycles),
("cycles", "Cycles (before, after or patterns):" ~
"&lt;CYCLE | &gt;CYCLE | GLOB ..." | safe , cycles),
("tasks", "Task Name Globs", tasks),
] -%}
<div class="form-group col-sm-12 col-md-6">
Expand Down Expand Up @@ -107,28 +107,28 @@
<select id="order" name="order" title="Sort Order"
class="form-control">
{% for k, v in [
("time_desc", "new-&gt;old"),
("time_asc", "old-&gt;new"),
("cycle_desc_name_asc", "new-&gt;old cycle, a-z task name"),
("cycle_desc_name_desc", "new-&gt;old cycle, z-a task name"),
("cycle_asc_name_asc", "old-&gt;new cycle, a-z task name"),
("cycle_asc_name_desc", "old-&gt;new cycle, z-a task name"),
("name_asc_cycle_desc", "a-z task name, new-&gt;old cycle"),
("name_desc_cycle_desc", "z-a task name, new-&gt;old cycle"),
("name_asc_cycle_asc", "a-z task name, old-&gt;new cycle"),
("name_desc_cycle_asc", "z-a task name, old-&gt;new cycle"),
("time_submit_desc", "submit time, new-&gt;old"),
("time_submit_asc", "submit time, old-&gt;new"),
("time_run_desc", "run start time, new-&gt;old"),
("time_run_asc", "run start time, old-&gt;new"),
("time_run_exit_desc", "run exit time, new-&gt;old"),
("time_run_exit_asc", "run exit time, old-&gt;new"),
("duration_queue_desc", "queue duration, long-&gt;short"),
("duration_queue_asc", "queue duration, short-&gt;long"),
("duration_run_desc", "run duration, long-&gt;short"),
("duration_run_asc", "run duration, short-&gt;long"),
("duration_queue_run_desc", "queue+run duration, long-&gt;short"),
("duration_queue_run_asc", "queue+run duration, short-&gt;long"),
("time_desc", "new-&gt;old" | safe ),
("time_asc", "old-&gt;new" | safe ),
("cycle_desc_name_asc", "new-&gt;old cycle, a-z task name" | safe ),
("cycle_desc_name_desc", "new-&gt;old cycle, z-a task name" | safe ),
("cycle_asc_name_asc", "old-&gt;new cycle, a-z task name" | safe ),
("cycle_asc_name_desc", "old-&gt;new cycle, z-a task name" | safe ),
("name_asc_cycle_desc", "a-z task name, new-&gt;old cycle" | safe ),
("name_desc_cycle_desc", "z-a task name, new-&gt;old cycle" | safe ),
("name_asc_cycle_asc", "a-z task name, old-&gt;new cycle" | safe ),
("name_desc_cycle_asc", "z-a task name, old-&gt;new cycle" | safe ),
("time_submit_desc", "submit time, new-&gt;old" | safe ),
("time_submit_asc", "submit time, old-&gt;new" | safe ),
("time_run_desc", "run start time, new-&gt;old" | safe ),
("time_run_asc", "run start time, old-&gt;new" | safe ),
("time_run_exit_desc", "run exit time, new-&gt;old" | safe ),
("time_run_exit_asc", "run exit time, old-&gt;new" | safe ),
("duration_queue_desc", "queue duration, long-&gt;short" | safe ),
("duration_queue_asc", "queue duration, short-&gt;long" | safe ),
("duration_run_desc", "run duration, long-&gt;short" | safe ),
("duration_run_asc", "run duration, short-&gt;long" | safe ),
("duration_queue_run_desc", "queue+run duration, long-&gt;short" | safe ),
("duration_queue_run_asc", "queue+run duration, short-&gt;long" | safe ),
] -%}
<option
{% if order and order == k -%}
Expand Down
4 changes: 2 additions & 2 deletions lib/cylc/cylc-review/template/view.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ <h1>{{path}}</h1>
'WARNING', '<span class="nocode text-warning">WARNING</span>') -%}
{% endif -%}
{% if mode == "tags" -%}
{{line}}
{{line|safe}}
{% else -%}
{{line|urlise}}
{{ (line|urlise) | safe }}
{% endif -%}
{% endfor -%}
</pre>
Expand Down
9 changes: 7 additions & 2 deletions lib/cylc/review.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,13 @@ def __init__(self, *args, **kwargs):
if self.host_name and "." in self.host_name:
self.host_name = self.host_name.split(".", 1)[0]
self.cylc_version = CYLC_VERSION
template_env = jinja2.Environment(loader=jinja2.FileSystemLoader(
get_util_home("lib", "cylc", "cylc-review", "template")))
# Autoescape markup to prevent code injection from user inputs.
template_env = jinja2.Environment(
loader=jinja2.FileSystemLoader(
get_util_home("lib", "cylc", "cylc-review", "template")),
autoescape=jinja2.select_autoescape(
enabled_extensions=('html', 'xml'), default_for_string=True),
)
template_env.filters['urlise'] = self.url2hyperlink
self.template_env = template_env

Expand Down