[WebUI] Move HTML entity encoding to client

We should not be mangling the torrent data in the JSON API since this
can have unintended consquences with names and filepaths that can be
edited. If we escape those symbols in the JSON API then the data no
longer matches that stored by core. Therefore shift the encoding to the
client and consider dealing separetely with these entities when the user
first adds a torrent.

* Created a modified htmlEncode in Deluge Formatter based on extjs
method that also encodes single quotes.
* Removed renderers in ListViews since only templates specified via tpl
are used and any render attribute specified was a no-op.
* Removed old buggy escapeHtml

Resolves: https://dev.deluge-torrent.org/ticket/3459
Ref: https://docs.sencha.com/extjs/6.5.3/modern/src/String.js.html#Ext.String-method-htmlEncode
Ref: https://docs.sencha.com/extjs/3.4.0/source/Format.html#Ext-util-Format-method-htmlEncode
This commit is contained in:
Calum Lind 2022-02-14 10:23:19 +00:00
parent a5503c0c60
commit 8ece036770
No known key found for this signature in database
GPG Key ID: 90597A687B836BA3
5 changed files with 165 additions and 179 deletions

View File

@ -25,11 +25,6 @@ Ext.state.Manager.setProvider(
// Add some additional functions to ext and setup some of the // Add some additional functions to ext and setup some of the
// configurable parameters // configurable parameters
Ext.apply(Ext, { Ext.apply(Ext, {
escapeHTML: function (text) {
text = String(text).replace('<', '&lt;').replace('>', '&gt;');
return text.replace('&', '&amp;');
},
isObjectEmpty: function (obj) { isObjectEmpty: function (obj) {
for (var i in obj) { for (var i in obj) {
return false; return false;

View File

@ -15,7 +15,23 @@
* @version 1.3 * @version 1.3
* @singleton * @singleton
*/ */
Deluge.Formatters = { Deluge.Formatters = (function () {
var charToEntity = {
'&': '&amp;',
'>': '&gt;',
'<': '&lt;',
'"': '&quot;',
"'": '&#39;',
};
var charToEntityRegex = new RegExp(
'(' + Object.keys(charToEntity).join('|') + ')',
'g'
);
var htmlEncodeReplaceFn = function (match, capture) {
return charToEntity[capture];
};
/** /**
* Formats a date string in the date representation of the current locale, * Formats a date string in the date representation of the current locale,
* based on the systems timezone. * based on the systems timezone.
@ -24,6 +40,7 @@ Deluge.Formatters = {
* @return {String} a string in the date representation of the current locale * @return {String} a string in the date representation of the current locale
* or "" if seconds < 0. * or "" if seconds < 0.
*/ */
return (Formatters = {
date: function (timestamp) { date: function (timestamp) {
function zeroPad(num, count) { function zeroPad(num, count) {
var numZeropad = num + ''; var numZeropad = num + '';
@ -171,7 +188,14 @@ Deluge.Formatters = {
cssClassEscape: function (value) { cssClassEscape: function (value) {
return value.toLowerCase().replace('.', '_'); return value.toLowerCase().replace('.', '_');
}, },
};
htmlEncode: function (value) {
return !value
? value
: String(value).replace(charToEntityRegex, htmlEncodeReplaceFn);
},
});
})();
var fsize = Deluge.Formatters.size; var fsize = Deluge.Formatters.size;
var fsize_short = Deluge.Formatters.sizeShort; var fsize_short = Deluge.Formatters.sizeShort;
var fspeed = Deluge.Formatters.speed; var fspeed = Deluge.Formatters.speed;
@ -179,3 +203,4 @@ var ftime = Deluge.Formatters.timeRemaining;
var fdate = Deluge.Formatters.date; var fdate = Deluge.Formatters.date;
var fplain = Deluge.Formatters.plain; var fplain = Deluge.Formatters.plain;
Ext.util.Format.cssClassEscape = Deluge.Formatters.cssClassEscape; Ext.util.Format.cssClassEscape = Deluge.Formatters.cssClassEscape;
Ext.util.Format.htmlEncode = Deluge.Formatters.htmlEncode;

View File

@ -64,20 +64,6 @@ Deluge.add.AddWindow = Ext.extend(Deluge.add.Window, {
this.addButton(_('Cancel'), this.onCancelClick, this); this.addButton(_('Cancel'), this.onCancelClick, this);
this.addButton(_('Add'), this.onAddClick, this); this.addButton(_('Add'), this.onAddClick, this);
function torrentRenderer(value, p, r) {
if (r.data['info_hash']) {
return String.format(
'<div class="x-deluge-add-torrent-name">{0}</div>',
value
);
} else {
return String.format(
'<div class="x-deluge-add-torrent-name-loading">{0}</div>',
value
);
}
}
this.list = new Ext.list.ListView({ this.list = new Ext.list.ListView({
store: new Ext.data.SimpleStore({ store: new Ext.data.SimpleStore({
fields: [ fields: [
@ -91,7 +77,6 @@ Deluge.add.AddWindow = Ext.extend(Deluge.add.Window, {
id: 'torrent', id: 'torrent',
width: 150, width: 150,
sortable: true, sortable: true,
renderer: torrentRenderer,
dataIndex: 'text', dataIndex: 'text',
tpl: new Ext.XTemplate( tpl: new Ext.XTemplate(
'<div class="x-deluge-add-torrent-name">{text:htmlEncode}</div>' '<div class="x-deluge-add-torrent-name">{text:htmlEncode}</div>'

View File

@ -46,7 +46,6 @@ Deluge.preferences.PreferencesWindow = Ext.extend(Ext.Window, {
columns: [ columns: [
{ {
id: 'name', id: 'name',
renderer: fplain,
dataIndex: 'name', dataIndex: 'name',
}, },
], ],

View File

@ -13,7 +13,6 @@ import shutil
import tempfile import tempfile
from base64 import b64encode from base64 import b64encode
from types import FunctionType from types import FunctionType
from xml.sax.saxutils import escape as xml_escape
from twisted.internet import defer, reactor from twisted.internet import defer, reactor
from twisted.internet.defer import Deferred, DeferredList from twisted.internet.defer import Deferred, DeferredList
@ -375,8 +374,6 @@ class WebApi(JSONComponent):
methods available from the core RPC. methods available from the core RPC.
""" """
XSS_VULN_KEYS = ['name', 'message', 'comment', 'tracker_status', 'peers']
def __init__(self): def __init__(self):
super().__init__('Web', depend=['SessionProxy']) super().__init__('Web', depend=['SessionProxy'])
self.hostlist = HostList() self.hostlist = HostList()
@ -581,7 +578,7 @@ class WebApi(JSONComponent):
paths = [] paths = []
info = {} info = {}
for index, torrent_file in enumerate(files): for index, torrent_file in enumerate(files):
path = xml_escape(torrent_file['path']) path = torrent_file['path']
paths.append(path) paths.append(path)
torrent_file['progress'] = file_progress[index] torrent_file['progress'] = file_progress[index]
torrent_file['priority'] = file_priorities[index] torrent_file['priority'] = file_priorities[index]
@ -618,25 +615,10 @@ class WebApi(JSONComponent):
file_tree.walk(walk) file_tree.walk(walk)
d.callback(file_tree.get_tree()) d.callback(file_tree.get_tree())
def _on_torrent_status(self, torrent, d):
for key in self.XSS_VULN_KEYS:
try:
if key == 'peers':
for peer in torrent[key]:
peer['client'] = xml_escape(peer['client'])
else:
torrent[key] = xml_escape(torrent[key])
except KeyError:
pass
d.callback(torrent)
@export @export
def get_torrent_status(self, torrent_id, keys): def get_torrent_status(self, torrent_id, keys):
"""Get the status for a torrent, filtered by status keys.""" """Get the status for a torrent, filtered by status keys."""
main_deferred = Deferred() return component.get('SessionProxy').get_torrent_status(torrent_id, keys)
d = component.get('SessionProxy').get_torrent_status(torrent_id, keys)
d.addCallback(self._on_torrent_status, main_deferred)
return main_deferred
@export @export
def get_torrent_files(self, torrent_id): def get_torrent_files(self, torrent_id):