Skip to content
This repository has been archived by the owner on Jan 7, 2018. It is now read-only.
Permalink
Browse files Browse the repository at this point in the history
Fix XSS escaping issues in the admin data tables.
I mistakenly thought DataTables escaped things by default, but this is
not the case. Escape all data tables fields now and add capybara
integrations tests to ensure this is effective.

Setting up the Capybara tests required a slight change in the analytics
"Filter Logs" table, since the ajax request needs to be performed via
POST instead of GET. I think this is due to the length of the URL
involved in the query. This had worked in most browsers, but I'm
guessing this was broken in IE anyway, so it's probably a good idea
anyway.
  • Loading branch information
GUI committed Apr 10, 2015
1 parent 7e54cc6 commit f53a9fb
Show file tree
Hide file tree
Showing 13 changed files with 148 additions and 63 deletions.
30 changes: 30 additions & 0 deletions app/assets/javascripts/admin/app.js
Expand Up @@ -363,3 +363,33 @@ Admin.Save = Ember.Mixin.create({
}, this));
},
});

Admin.DataTablesHelpers = {
renderEscaped: function(value, type) {
if(type === 'display' && value) {
return _.escape(value);
}

return value;
},

renderListEscaped: function(value, type) {
if(type === 'display' && value) {
if(_.isArray(value)) {
return _.map(value, function(v) { return _.escape(v); }).join('<br>');
} else {
return _.escape(value);
}
}

return value;
},

renderTime: function(value, type) {
if(type === 'display' && value && value !== '-') {
return moment(value).format('YYYY-MM-DD HH:mm:ss');
}

return value;
},
};
16 changes: 2 additions & 14 deletions app/assets/javascripts/admin/views/admin_groups/table_view.js
Expand Up @@ -26,25 +26,13 @@ Admin.AdminGroupsTableView = Ember.View.extend({
{
data: 'api_scope_display_names',
title: 'API Scopes',
render: function(names, type) {
if(type === 'display' && names && names !== '-') {
return names.join('<br>');
}

return names;
}
render: Admin.DataTablesHelpers.renderListEscaped,
},
{
data: 'permission_display_names',
title: 'Access',
defaultContent: '-',
render: function(names, type) {
if(type === 'display' && names && names !== '-') {
return names.join('<br>');
}

return names;
}
render: Admin.DataTablesHelpers.renderListEscaped,
}
]
});
Expand Down
26 changes: 5 additions & 21 deletions app/assets/javascripts/admin/views/admins/table_view.js
Expand Up @@ -27,48 +27,32 @@ Admin.AdminsTableView = Ember.View.extend({
data: 'email',
title: 'E-mail',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'name',
title: 'Name',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'group_names',
title: 'Groups',
render: function(names, type) {
if(type === 'display' && names && names !== '-') {
return names.join('<br>');
}

return names;
}
render: Admin.DataTablesHelpers.renderListEscaped,
},
{
data: 'last_sign_in_at',
type: 'date',
title: 'Last Signed In',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}

return time;
},
render: Admin.DataTablesHelpers.renderTime,
},
{
data: 'created_at',
type: 'date',
title: 'Created',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}

return time;
},
render: Admin.DataTablesHelpers.renderTime,
}
]
});
Expand Down
2 changes: 2 additions & 0 deletions app/assets/javascripts/admin/views/api_scopes/table_view.js
Expand Up @@ -27,11 +27,13 @@ Admin.ApiScopesTableView = Ember.View.extend({
data: 'host',
title: 'Host',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'path_prefix',
title: 'Path Prefix',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
}
]
});
Expand Down
13 changes: 6 additions & 7 deletions app/assets/javascripts/admin/views/api_users/table_view.js
Expand Up @@ -27,40 +27,39 @@ Admin.ApiUsersTableView = Ember.View.extend({
data: 'first_name',
title: 'First Name',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'last_name',
title: 'Last Name',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'use_description',
title: 'Purpose',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'created_at',
type: 'date',
title: 'Created',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}

return time;
},
render: Admin.DataTablesHelpers.renderTime,
},
{
data: 'registration_source',
title: 'Registration Source',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'api_key_preview',
title: 'API Key',
defaultContent: '-',
orderable: false,
render: Admin.DataTablesHelpers.renderEscaped,
},
]
});
Expand Down
3 changes: 3 additions & 0 deletions app/assets/javascripts/admin/views/apis/table_view.js
Expand Up @@ -39,17 +39,20 @@ Admin.ApisTableView = Ember.View.extend({
data: 'frontend_host',
title: 'Host',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'frontend_prefixes',
title: 'Prefixes',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'sort_order',
title: 'Matching Order',
defaultContent: '-',
width: 130,
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: null,
Expand Down
22 changes: 15 additions & 7 deletions app/assets/javascripts/admin/views/stats/logs_table_view.js
Expand Up @@ -9,6 +9,9 @@ Admin.LogsTableView = Ember.View.extend({
serverSide: true,
ajax: {
url: '/admin/stats/logs.json',
// Use POST for this endpoint, since the URLs can be very long and
// exceed URL length limits in IE (and apparently Capybara too).
type: 'POST',
data: _.bind(function(data) {
var query = this.get('controller.query.params');
return _.extend({}, data, query);
Expand Down Expand Up @@ -43,28 +46,25 @@ Admin.LogsTableView = Ember.View.extend({
type: 'date',
title: 'Time',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}

return time;
},
render: Admin.DataTablesHelpers.renderTime,
},
{
data: 'request_method',
title: 'Method',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_host',
title: 'Host',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_url',
title: 'URL',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'user_email',
Expand All @@ -86,26 +86,31 @@ Admin.LogsTableView = Ember.View.extend({
data: 'request_ip',
title: 'IP Address',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_ip_country',
title: 'Country',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_ip_region',
title: 'State',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_ip_city',
title: 'City',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'response_status',
title: 'Status',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'response_time',
Expand All @@ -123,16 +128,19 @@ Admin.LogsTableView = Ember.View.extend({
data: 'response_content_type',
title: 'Content Type',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_accept_encoding',
title: 'Accept Encoding',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'request_user_agent',
title: 'User Agent',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
]
});
Expand Down
17 changes: 5 additions & 12 deletions app/assets/javascripts/admin/views/stats/users_table_view.js
Expand Up @@ -36,22 +36,20 @@ Admin.StatsUsersTableView = Ember.View.extend({
data: 'first_name',
title: 'First Name',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'last_name',
title: 'Last Name',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderEscaped,
},
{
data: 'created_at',
type: 'date',
title: 'Signed Up',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}
},
render: Admin.DataTablesHelpers.renderTime,
},
{
data: 'hits',
Expand All @@ -70,18 +68,13 @@ Admin.StatsUsersTableView = Ember.View.extend({
type: 'date',
title: 'Last Request',
defaultContent: '-',
render: function(time, type) {
if(type === 'display' && time && time !== '-') {
return moment(time).format('YYYY-MM-DD HH:mm:ss');
}

return time;
},
render: Admin.DataTablesHelpers.renderTime,
},
{
data: 'use_description',
title: 'Use Description',
defaultContent: '-',
render: Admin.DataTablesHelpers.renderTime,
},
]
});
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Expand Up @@ -71,6 +71,7 @@
collection do
get "search"
get "logs"
post "logs"
get "users"
get "map"
end
Expand Down
7 changes: 7 additions & 0 deletions spec/factories/api_users.rb
Expand Up @@ -11,5 +11,12 @@
factory :invalid_api_user do
terms_and_conditions ""
end

factory :xss_api_user do
first_name '"><script class="xss-test">alert("Hello first_name");</script>'
last_name '"><script class="xss-test">alert("Hello last_name");</script>'
use_description '"><script class="xss-test">alert("Hello use_description");</script>'
registration_source '"><script class="xss-test">alert("Hello registration_source");</script>'
end
end
end

0 comments on commit f53a9fb

Please sign in to comment.