From 661d3d28e97bb49bef075c0314edad5879148aaa Mon Sep 17 00:00:00 2001 From: Gusted Date: Sun, 16 Jan 2022 05:14:32 +0000 Subject: [PATCH] Prevent possible XSS when using jQuery (#18289) In the case of misuse or misunderstanding from a developer whereby, if `sel` can receive user-controlled data, jQuery `$(sel)` can lead to the creation of a new element. Current usage is using hard-coded selectors in the templates, but nobody prevents that from expanding to user-controlled somehow. --- .../doc/developers/guidelines-frontend.md | 5 ++++ .../js/components/RepoBranchTagDropdown.js | 2 +- web_src/js/features/common-global.js | 14 ++++----- web_src/js/features/comp/LabelEdit.js | 2 +- web_src/js/features/repo-branch.js | 2 +- web_src/js/features/repo-common.js | 6 ++-- web_src/js/features/repo-diff.js | 2 +- web_src/js/features/repo-issue.js | 4 +-- web_src/js/features/repo-legacy.js | 30 +++++++++---------- web_src/js/features/repo-settings.js | 6 ++-- 10 files changed, 39 insertions(+), 34 deletions(-) diff --git a/docs/content/doc/developers/guidelines-frontend.md b/docs/content/doc/developers/guidelines-frontend.md index 9fec5bd17e..cbd2ca8a24 100644 --- a/docs/content/doc/developers/guidelines-frontend.md +++ b/docs/content/doc/developers/guidelines-frontend.md @@ -127,3 +127,8 @@ We forbid `dataset` usage, its camel-casing behaviour makes it hard to grep for ### Vue2/Vue3 and JSX Gitea is using Vue2 now, we plan to upgrade to Vue3. We decided not to introduce JSX to keep the HTML and the JavaScript code separated. + +### jQuery's `$(...)` + +jQuery's `$` function has a broad functionality depending on the input. Well, this can be seen as nice, it's also a fallpit for possible XSS attacks when the input is user-controlled. +The usage of the function can be correct in certain situations, but it is discourage and recommended to use a more specific function of jQuery(e.g. `$.find`, `$.parseHTML`). diff --git a/web_src/js/components/RepoBranchTagDropdown.js b/web_src/js/components/RepoBranchTagDropdown.js index 50c71d5bac..2b260e9399 100644 --- a/web_src/js/components/RepoBranchTagDropdown.js +++ b/web_src/js/components/RepoBranchTagDropdown.js @@ -2,7 +2,7 @@ import Vue from 'vue'; import {vueDelimiters} from './VueComponentLoader.js'; export function initRepoBranchTagDropdown(selector) { - $(selector).each(function () { + $.find(selector).each(function () { const $dropdown = $(this); const $data = $dropdown.find('.data'); const data = { diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js index bf9d21ac49..258a056e32 100644 --- a/web_src/js/features/common-global.js +++ b/web_src/js/features/common-global.js @@ -124,7 +124,7 @@ export function initGlobalCommon() { $('.tabable.menu .item').tab(); $('.toggle.button').on('click', function () { - $($(this).data('target')).slideToggle(100); + $.find($(this).data('target')).slideToggle(100); }); // make table and elements clickable like a link @@ -202,7 +202,7 @@ export function initGlobalLinkActions() { closable: false, onApprove() { if ($this.data('type') === 'form') { - $($this.data('form')).trigger('submit'); + $.find($this.data('form')).trigger('submit'); return; } @@ -240,7 +240,7 @@ export function initGlobalLinkActions() { closable: false, onApprove() { if ($this.data('type') === 'form') { - $($this.data('form')).trigger('submit'); + $.find($this.data('form')).trigger('submit'); return; } @@ -293,7 +293,7 @@ export function initGlobalLinkActions() { export function initGlobalButtons() { $('.show-panel.button').on('click', function () { - $($(this).data('panel')).show(); + $.find($(this).data('panel')).show(); }); $('.hide-panel.button').on('click', function (event) { @@ -301,7 +301,7 @@ export function initGlobalButtons() { event.preventDefault(); let sel = $(this).attr('data-panel'); if (sel) { - $(sel).hide(); + $.find(sel).hide(); return; } sel = $(this).attr('data-panel-closest'); @@ -314,8 +314,8 @@ export function initGlobalButtons() { }); $('.show-modal.button').on('click', function () { - $($(this).data('modal')).modal('show'); - const colorPickers = $($(this).data('modal')).find('.color-picker'); + $.find($(this).data('modal')).modal('show'); + const colorPickers = $.find($(this).data('modal')).find('.color-picker'); if (colorPickers.length > 0) { initCompColorPicker(); } diff --git a/web_src/js/features/comp/LabelEdit.js b/web_src/js/features/comp/LabelEdit.js index 7d71e6effa..7c31080be8 100644 --- a/web_src/js/features/comp/LabelEdit.js +++ b/web_src/js/features/comp/LabelEdit.js @@ -1,7 +1,7 @@ import {initCompColorPicker} from './ColorPicker.js'; export function initCompLabelEdit(selector) { - if (!$(selector).length) return; + if (!$.find(selector).length) return; // Create label const $newLabelPanel = $('.new-label.segment'); $('.new-label.button').on('click', () => { diff --git a/web_src/js/features/repo-branch.js b/web_src/js/features/repo-branch.js index 4402411bfd..1dddbf7276 100644 --- a/web_src/js/features/repo-branch.js +++ b/web_src/js/features/repo-branch.js @@ -2,6 +2,6 @@ export function initRepoBranchButton() { $('.show-create-branch-modal.button').on('click', function () { $('#create-branch-form')[0].action = $('#create-branch-form').data('base-action') + $(this).data('branch-from-urlcomponent'); $('#modal-create-branch-from-span').text($(this).data('branch-from')); - $($(this).data('modal')).modal('show'); + $.find($(this).data('modal')).modal('show'); }); } diff --git a/web_src/js/features/repo-common.js b/web_src/js/features/repo-common.js index 3ddabe10f1..712086952d 100644 --- a/web_src/js/features/repo-common.js +++ b/web_src/js/features/repo-common.js @@ -65,18 +65,18 @@ export function initRepoClone() { } export function initRepoCommonBranchOrTagDropdown(selector) { - $(selector).each(function () { + $.find(selector).each(function () { const $dropdown = $(this); $dropdown.find('.reference.column').on('click', function () { $dropdown.find('.scrolling.reference-list-menu').hide(); - $($(this).data('target')).show(); + $.find($(this).data('target')).show(); return false; }); }); } export function initRepoCommonFilterSearchDropdown(selector) { - const $dropdown = $(selector); + const $dropdown = $.find(selector); $dropdown.dropdown({ fullTextSearch: true, selectOnKeydown: false, diff --git a/web_src/js/features/repo-diff.js b/web_src/js/features/repo-diff.js index 3d937bbdb1..4f16133b5f 100644 --- a/web_src/js/features/repo-diff.js +++ b/web_src/js/features/repo-diff.js @@ -15,7 +15,7 @@ export function initRepoDiffFileViewToggle() { $this.parent().children().removeClass('active'); $this.addClass('active'); - const $target = $($this.data('toggle-selector')); + const $target = $.find($this.data('toggle-selector')); $target.parent().children().addClass('hide'); $target.removeClass('hide'); }); diff --git a/web_src/js/features/repo-issue.js b/web_src/js/features/repo-issue.js index 6e57facfd2..b062c44675 100644 --- a/web_src/js/features/repo-issue.js +++ b/web_src/js/features/repo-issue.js @@ -28,7 +28,7 @@ export function initRepoIssueTimeTracking() { }); $(document).on('click', 'button.issue-delete-time', function () { const sel = `.issue-delete-time-modal[data-id="${$(this).data('id')}"]`; - $(sel).modal({ + $.find(sel).modal({ duration: 200, onApprove() { $(`${sel} form`).trigger('submit'); @@ -535,7 +535,7 @@ export function initRepoIssueReferenceIssue() { const content = $(`#comment-${$this.data('target')}`).text(); const poster = $this.data('poster-username'); const reference = $this.data('reference'); - const $modal = $($this.data('modal')); + const $modal = $.find($this.data('modal')); $modal.find('textarea[name="content"]').val(`${content}\n\n_Originally posted by @${poster} in ${reference}_`); $modal.modal('show'); diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index f30345bfee..5a42ac7620 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -42,7 +42,7 @@ export function initRepoCommentForm() { $branchMenu.find('.item:not(.no-select)').click(function () { const selectedValue = $(this).data('id'); const editMode = $('#editing_mode').val(); - $($(this).data('id-selector')).val(selectedValue); + $.find($(this).data('id-selector')).val(selectedValue); if ($isNewIssue) { $selectBranch.find('.ui .branch-name').text($(this).data('name')); return; @@ -58,7 +58,7 @@ export function initRepoCommentForm() { $selectBranch.find('.reference.column').on('click', function () { $selectBranch.find('.scrolling.reference-list-menu').css('display', 'none'); $selectBranch.find('.reference .text').removeClass('black'); - $($(this).data('target')).css('display', 'block'); + $.find($(this).data('target')).css('display', 'block'); $(this).find('.text').addClass('black'); return false; }); @@ -145,9 +145,9 @@ export function initRepoCommentForm() { $(this).parent().find('.item').each(function () { if ($(this).hasClass('checked')) { listIds.push($(this).data('id')); - $($(this).data('id-selector')).removeClass('hide'); + $.find($(this).data('id-selector')).removeClass('hide'); } else { - $($(this).data('id-selector')).addClass('hide'); + $.find($(this).data('id-selector')).addClass('hide'); } }); if (listIds.length === 0) { @@ -155,7 +155,7 @@ export function initRepoCommentForm() { } else { $noSelect.addClass('hide'); } - $($(this).parent().data('id')).val(listIds.join(',')); + $.find($(this).parent().data('id')).val(listIds.join(',')); return false; }); $listMenu.find('.no-select.item').on('click', function (e) { @@ -182,7 +182,7 @@ export function initRepoCommentForm() { $(this).addClass('hide'); }); $noSelect.removeClass('hide'); - $($(this).parent().data('id')).val(''); + $.find($(this).parent().data('id')).val(''); }); } @@ -247,7 +247,7 @@ export function initRepoCommentForm() { $list.find('.selected').html(''); $list.find('.no-select').removeClass('hide'); - $(input_id).val(''); + $.find(input_id).val(''); }); } @@ -450,20 +450,20 @@ export function initRepository() { // Enable or select internal/external wiki system and issue tracker. $('.enable-system').on('change', function () { if (this.checked) { - $($(this).data('target')).removeClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); + $.find($(this).data('target')).removeClass('disabled'); + if (!$(this).data('context')) $.find($(this).data('context')).addClass('disabled'); } else { - $($(this).data('target')).addClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); + $.find($(this).data('target')).addClass('disabled'); + if (!$(this).data('context')) $.find($(this).data('context')).removeClass('disabled'); } }); $('.enable-system-radio').on('change', function () { if (this.value === 'false') { - $($(this).data('target')).addClass('disabled'); - if (typeof $(this).data('context') !== 'undefined') $($(this).data('context')).removeClass('disabled'); + $.find($(this).data('target')).addClass('disabled'); + if (typeof $(this).data('context') !== 'undefined') $.find($(this).data('context')).removeClass('disabled'); } else if (this.value === 'true') { - $($(this).data('target')).removeClass('disabled'); - if (typeof $(this).data('context') !== 'undefined') $($(this).data('context')).addClass('disabled'); + $.find($(this).data('target')).removeClass('disabled'); + if (typeof $(this).data('context') !== 'undefined') $.find($(this).data('context')).addClass('disabled'); } }); } diff --git a/web_src/js/features/repo-settings.js b/web_src/js/features/repo-settings.js index b0d43bd487..9568d1e80d 100644 --- a/web_src/js/features/repo-settings.js +++ b/web_src/js/features/repo-settings.js @@ -52,14 +52,14 @@ export function initRepoSettingBranches() { initRepoCommonFilterSearchDropdown('.protected-branches .dropdown'); $('.enable-protection, .enable-whitelist, .enable-statuscheck').on('change', function () { if (this.checked) { - $($(this).data('target')).removeClass('disabled'); + $.find($(this).data('target')).removeClass('disabled'); } else { - $($(this).data('target')).addClass('disabled'); + $.find($(this).data('target')).addClass('disabled'); } }); $('.disable-whitelist').on('change', function () { if (this.checked) { - $($(this).data('target')).addClass('disabled'); + $.find($(this).data('target')).addClass('disabled'); } }); }