Skip to content

Commit

Permalink
improve uri encoding
Browse files Browse the repository at this point in the history
make sure uri values are encoded with uri filter not html
  • Loading branch information
sni committed Sep 16, 2024
1 parent ca57724 commit 3160e7a
Show file tree
Hide file tree
Showing 13 changed files with 26 additions and 20 deletions.
8 changes: 4 additions & 4 deletions plugins/plugins-available/agents/templates/agents.tt
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
[% END %]
<td class="truncate w-60 max-w-0 js-agent-row">
<div class="flexrow flex-nowrap gap-x-1 justify-between">
<a class="link" href="agents.cgi?action=edit&amp;hostname=[% h.name | uri %]&backend=[% h.peer_key | html %]" title="edit this host">[% h.name | html %]</a>
<a class="link" href="agents.cgi?action=edit&amp;hostname=[% h.name | uri %]&backend=[% h.peer_key | uri %]" title="edit this host">[% h.name | html %]</a>
<a href="status.cgi?host=[% h.name | uri %]" title="view service details for this host"><i class="fa-solid fa-bars"></i></a>
</div>
</td>
Expand All @@ -81,7 +81,7 @@
[% IF state == "OK" %]
<td>
<div class="flexrow flex-nowrap gap-x-1">
<div class="w-5 h-5 m-0 p-0 min-w-fit"><img src="[% url_prefix %]plugins/[% plugin_name %]/images/[% agent.icon | html %]" alt="[% agent.type | html %]" width=25 height=25></div>
<div class="w-5 h-5 m-0 p-0 min-w-fit"><img src="[% url_prefix %]plugins/[% plugin_name %]/images/[% agent.icon | uri %]" alt="[% agent.type | html %]" width=25 height=25></div>
[% IF info.exists(hostname) %]<div class="w-full h-full" title="[% info.$hostname.full_version | html %]">[% info.$hostname.version | html %]</div>[% END %]
</div>
</td>
Expand Down Expand Up @@ -126,7 +126,7 @@
</td>
[% ELSE %]
<td colspan="7">
<a class="link" href="extinfo.cgi?type=2&host=[% h.name | uri %]&service=agent+version&backend=[% h.peer_key | html %]" title="view agent version service">
<a class="link" href="extinfo.cgi?type=2&host=[% h.name | uri %]&service=agent+version&backend=[% h.peer_key | uri %]" title="view agent version service">
<div class="flexrow flex-nowrap gap-x-1 justify-between">
<div>[% info.$hostname.plugin_output.replace("^" _ state _ "(: | - )", "") %]</div>
[% IF info.$hostname.last_state_change %]<div title="[% date_format(c, info.$hostname.last_state_change) %]">(for [% duration(date.now - info.$hostname.last_state_change, 6) %])</div>[% END %]
Expand All @@ -149,7 +149,7 @@
<input type="hidden" name="CSRFtoken" value="[% get_user_token(c) %]">
<a href="#" class="js-update-btn" onclick="return send_form_in_background_and_reload(this)"><i class="fa-solid fa-arrows-rotate text-sm" title="Update inventory"></i></a>
</form>
<a href="agents.cgi?action=edit&amp;hostname=[% h.name | uri %]&backend=[% h.peer_key | html %]"><i class="fa-solid fa-pencil text-sm" title='Edit this host'></i></a>
<a href="agents.cgi?action=edit&amp;hostname=[% h.name | uri %]&backend=[% h.peer_key | uri %]"><i class="fa-solid fa-pencil text-sm" title='Edit this host'></i></a>
[% PROCESS _button btn = {
form => { action => 'agents.cgi', },
data => { 'action' => 'remove', 'hostname' => h.name, backend => h.peer_key, CSRFtoken => get_user_token(c), },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</div>
[% ELSE %]
[% IF defaults.exists(key) && defaults.$key.exists('link') && defaults.$key.link == 'icon' %]
[% escape_html(value) %] <img class="not-clickable" src="[% logo_path_prefix %][% value | html %]" width="20" height="20" alt='[% key %]' title='[% key %]' style="vertical-align: middle;">
[% escape_html(value) %] <img class="not-clickable" src="[% logo_path_prefix %][% value | uri %]" width="20" height="20" alt='[% key %]' title='[% key %]' style="vertical-align: middle;">
[% ELSIF defaults.exists(key) && defaults.$key.exists('link') %]
<a class="link" href="conf.cgi?sub=objects&amp;type=[% defaults.$key.link %]&amp;data.name=[% value | uri %]">[% value | html %]</a>
[% ELSE %]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
[% IF has_contact %]Yes[% ELSE %]No[% END %]
[% IF backends_with_obj_config.keys.size > 0 %]
[% IF has_contact %]
- <a class="button w-full" href="conf.cgi?sub=objects&amp;type=contact&amp;data.name=[% user_name | html %]">edit in config tool</a>
- <a class="button w-full" href="conf.cgi?sub=objects&amp;type=contact&amp;data.name=[% user_name | uri %]">edit in config tool</a>
[% ELSE %]
- <a class="button w-full" href="conf.cgi?sub=objects&amp;action=new&amp;type=contact&amp;obj.contact_name=[% user_name | uri %]">create</a>
[% END %]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
%]
<div class="card w-full h-full flexcol flex-nowrap gap-0 overflow-hidden">
<div class="head justify-between">
<a href="[% IF c.req.parameters.exists('backlink') %][% c.req.parameters.backlink | html %][% ELSIF show_object && back == "edit" %]conf.cgi?sub=objects&amp;data.id=[% object.get_id() %][% ELSE %]conf.cgi?sub=objects&amp;action=browser#[% file_link | html %][% END %]" onclick="return confirm_discard_changes()" class="button header-button rounded w-[70px]" title="Go back">
<a href="[% IF c.req.parameters.exists('backlink') %][% c.req.parameters.backlink | html %][% ELSIF show_object && back == "edit" %]conf.cgi?sub=objects&amp;data.id=[% object.get_id() %][% ELSE %]conf.cgi?sub=objects&amp;action=browser#[% file_link | uri %][% END %]" onclick="return confirm_discard_changes()" class="button header-button rounded w-[70px]" title="Go back">
<i class="uil uil-angle-double-left"></i>Back
</a>
<h3>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
[% hash = l.hash %]
[% commit = blame.commits.$hash %]
<tr>
<td><a class="link" href="conf.cgi?sub=objects&action=history&id=[% hash | html %]">[% hash.substr(0, 6) %]</a></td>
<td><a class="link" href="conf.cgi?sub=objects&action=history&id=[% hash | uri %]">[% hash.substr(0, 6) %]</a></td>
<td><a class="link" href="mailto:[% commit.item('author-mail').replace('<', '').replace('>', '') %]">[% commit.author | html %]</a></td>
<td style="text-align: right;">[% date_format(c, commit.item('author-time')) %]</td>
<td title="[% commit.summary | html %]"><a class="link" href="conf.cgi?sub=objects&action=history&id=[% hash | html %]">[% commit.summary.substr(0, 30) | html %]</a></td>
<td title="[% commit.summary | html %]"><a class="link" href="conf.cgi?sub=objects&action=history&id=[% hash | uri %]">[% commit.summary.substr(0, 30) | html %]</a></td>
<td>[% l.sourceline %]</td>
<td><pre style="margin: 0; padding: 0;">[%- l.line | html -%]</pre></td>
</tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ var objects_type = "[% objects_type %]";
[% IF shortname.length > 15 %][% shortname = shortname.substr(0,15) _ '..' %][% END %]
[% filepath = o.file.display.replace(files_root,'/'); IF filepath.length > 50; filepath = filepath.substr(-50); END; filepath = '...' _ filepath %]
<div class="jstree-draggable clickable hoverable browser_file_item" onclick="window.location='conf.cgi?bare=1&amp;sub=objects&amp;type=[% type | html %]&amp;data.id=[% o.get_id() %]&amp;referer=[% as_url_arg(short_uri(c, {referer => 'undef'})) %]'" ondblclick="window.parent.location='conf.cgi?sub=objects&amp;type=[% type | html %]&amp;data.id=[% o.get_id() %]'" title="[% fullname %]">
<img src="../plugins/[% plugin_name %]/images/obj_[% type | html %].png" alt="config item">
<img src="../plugins/[% plugin_name %]/images/obj_[% type | uri %].png" alt="config item">
<div class="browser_file_shorttitle">[% shortname %]</div>
<div class="browser_file_longtitle">[% fullname %]</div>
<div class="browser_file_type" title="[% type | html %]">[% type | html %]</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@
<tr class="js-tabs" id="node_row_[% s.peer_key | html %]">
<td class="align-top js-node-row">[% IF s.section != "Default" && s.section != "" && s.section != last_section; s.section | html; END; last_section = s.section; %]</td>
<td class="align-top" title="[% s.peer_key | html %]">[% s.peer_name | html %]</td>
<td class="align-top">[% IF s.omd_site %]<a class="link" target="_blank" href="//[% s.host_name | html %]/[% s.omd_site | html %]/"><i class="uil uil-external-link-alt text-xs"></i>[% s.host_name | html %]</a>[% END %]</td>
<td class="align-top">[% IF s.omd_site %]<a class="link" target="_blank" href="//[% s.host_name | uri %]/[% s.omd_site | uri %]/"><i class="uil uil-external-link-alt text-xs"></i>[% s.host_name | html %]</a>[% END %]</td>
<td class="align-top">
[% IF s.omd_site %]
[% remote_thruk_url = get_remote_thruk_url(c, s.peer_key) %]
<a class="link" href="[% IF remote_thruk_url %]proxy.cgi/[% s.peer_key | html %][% remote_thruk_url %][% ELSE %][% url_prefix %][% END %]" target="_blank">
<a class="link" href="[% IF remote_thruk_url %]proxy.cgi/[% s.peer_key | uri %][% remote_thruk_url %][% ELSE %][% url_prefix %][% END %]" target="_blank">
<i class="fa-solid fa-desktop text-xs" title="Open site in a new tab."></i> [% s.omd_site | html %]
</a>
[% END %]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
[% FOR t = report_themes %]
[% IF t.name == r.params.report_theme %]
[% FOR s = t.reportstyles %]
<link rel="stylesheet" type="text/css" href="[% url_prefix %]themes/[% t.name | html %]/[% s | html %]" />
<link rel="stylesheet" type="text/css" href="[% url_prefix %]themes/[% t.name | uri %]/[% s | uri %]" />
[% END %]
[% END %]
[% END %]
Expand Down
8 changes: 7 additions & 1 deletion t/083-xss.t
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ sub check_templates {
if($key =~ m/^(href|src)$/mx) {
if($var =~ m/
\|\s*uri
|\|\s*html
# |\|\s*html # must not use html in urls, does not escpace plus properly
|escape_html\(
|full_uri\(
|short_uri\(
Expand All @@ -142,6 +142,12 @@ sub check_templates {
$escaped_keys->{$escaped_var} = $linenr;
next;
}

# special case is src/href as one string, this one can use html filter instead of uri as well
if($value =~ m/\|\s*html/mx && $value =~ m/^"\[%.*?%\]"$/mx) {
next;
}

fail(sprintf("%s:%d uses variable '%s' without uri/html filter in: %s=%s", $file, $linenr, $var, $key, $value));
$failed++;
}
Expand Down
2 changes: 1 addition & 1 deletion templates/_extinfo_host_service_details.tt
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ END
<tr>
<th class="align-top">[% IF type == "service" %]Service Groups[% ELSIF type == "host" %]Host Groups[% END %]</th>
<td class='whitespace-normal'><span style="width: 480px; display: block;">
[% FOREACH group IN obj.groups.sort %][% IF !loop.first() %], [% END %]<a class="link" href="status.cgi?[% type | html %]group=[% group | uri %]&amp;style=detail">[% group | html %]</a>[% END %]
[% FOREACH group IN obj.groups.sort %][% IF !loop.first() %], [% END %]<a class="link" href="status.cgi?[% type | uri %]group=[% group | uri %]&amp;style=detail">[% group | html %]</a>[% END %]
</span></td>
</tr>
[% END %]
Expand Down
2 changes: 1 addition & 1 deletion templates/_status_hostdetail_table.tt
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
<td class='[% class %] name'>
<div class="status col_host_name flex items-center min-w-max">
<div class="flex-1">
<a class="link" href="extinfo.cgi?type=1&amp;host=[% h.name | uri %]&amp;backend=[% h.peer_key | html %]" title="[% h.address | html %] - [% h.alias | html %]">[% _host(h) | html %]</a>
<a class="link" href="extinfo.cgi?type=1&amp;host=[% h.name | uri %]&amp;backend=[% h.peer_key | uri %]" title="[% h.address | html %] - [% h.alias | html %]">[% _host(h) | html %]</a>
</div>
[% PROCESS _status_host_attributes hostprefix="" host=h host_comment_count=comment_count with_status=1 %]
</div>
Expand Down
6 changes: 3 additions & 3 deletions templates/broadcast.tt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
[% FOREACH b = all_broadcasts %]
<tr>
<td>
<a href="broadcast.cgi?id=[% b.basefile | html %]&amp;action=edit">
<a href="broadcast.cgi?id=[% b.basefile | uri %]&amp;action=edit">
[% IF b.name %]
[% b.name | html %]
[% ELSE %]
Expand All @@ -55,10 +55,10 @@
<td class="max-w-sm truncate">[% b.contactgroups.join(', ') | html %]</td>
<td>
<div class="flex gap-1 justify-center">
<a href="broadcast.cgi?id=[% b.basefile | html %]&amp;action=clone">
<a href="broadcast.cgi?id=[% b.basefile | uri %]&amp;action=clone">
<i class="fa-solid fa-copy small" title='Clone Broadcast'></i>
</a>
<a href="broadcast.cgi?id=[% b.basefile | html %]&amp;action=edit">
<a href="broadcast.cgi?id=[% b.basefile | uri %]&amp;action=edit">
<i class="fa-solid fa-pencil small" title='Edit Broadcast'></i>
</a>
<form action="broadcast.cgi" method="POST">
Expand Down
2 changes: 1 addition & 1 deletion templates/user_profile.tt
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@
<td class="align-top">[% IF key.exists('created') %][% date_format(c, key.created) %][% ELSE %]unknown[% END %]</td>
<td class="align-top">[% IF key.exists('last_used') %][% date_format(c, key.last_used) %] ([% IF key.exists('last_from') %][% key.last_from | html %][% END %])[% ELSE %]never[% END %]</td>
<td class="align-top text-center">
<a href="user.cgi?file=[% basename(key.file) | html %]&amp;action=remove_key" onClick="if(confirm('Really remove?')) { setBtnSpinner(this); return true; } else { return false; }" title="delete this api key">
<a href="user.cgi?file=[% basename(key.file) | uri %]&amp;action=remove_key" onClick="if(confirm('Really remove?')) { setBtnSpinner(this); return true; } else { return false; }" title="delete this api key">
<i class="fa-solid fa-trash text-base"></i>
</a>
</td>
Expand Down

0 comments on commit 3160e7a

Please sign in to comment.