Commit e437f902 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-fix-xss-on-frequent-groups-dropdown-ee' into 'master'

Fix xss on frequent groups dropdown

Closes #15

See merge request gitlab-org/security/gitlab!50
parents f5f6e6e5 680eb56c
...@@ -5,7 +5,7 @@ import AccessorUtilities from '~/lib/utils/accessor'; ...@@ -5,7 +5,7 @@ import AccessorUtilities from '~/lib/utils/accessor';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
import store from '../store/'; import store from '../store/';
import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants'; import { FREQUENT_ITEMS, STORAGE_KEY } from '../constants';
import { isMobile, updateExistingFrequentItem } from '../utils'; import { isMobile, updateExistingFrequentItem, sanitizeItem } from '../utils';
import FrequentItemsSearchInput from './frequent_items_search_input.vue'; import FrequentItemsSearchInput from './frequent_items_search_input.vue';
import FrequentItemsList from './frequent_items_list.vue'; import FrequentItemsList from './frequent_items_list.vue';
import frequentItemsMixin from './frequent_items_mixin'; import frequentItemsMixin from './frequent_items_mixin';
...@@ -64,7 +64,9 @@ export default { ...@@ -64,7 +64,9 @@ export default {
this.fetchFrequentItems(); this.fetchFrequentItems();
} }
}, },
logItemAccess(storageKey, item) { logItemAccess(storageKey, unsanitizedItem) {
const item = sanitizeItem(unsanitizedItem);
if (!AccessorUtilities.isLocalStorageAccessSafe()) { if (!AccessorUtilities.isLocalStorageAccessSafe()) {
return false; return false;
} }
......
<script> <script>
import FrequentItemsListItem from './frequent_items_list_item.vue'; import FrequentItemsListItem from './frequent_items_list_item.vue';
import frequentItemsMixin from './frequent_items_mixin'; import frequentItemsMixin from './frequent_items_mixin';
import { sanitizeItem } from '../utils';
export default { export default {
components: { components: {
...@@ -48,6 +49,9 @@ export default { ...@@ -48,6 +49,9 @@ export default {
? this.translations.itemListErrorMessage ? this.translations.itemListErrorMessage
: this.translations.itemListEmptyMessage; : this.translations.itemListEmptyMessage;
}, },
sanitizedItems() {
return this.items.map(sanitizeItem);
},
}, },
}; };
</script> </script>
...@@ -59,7 +63,7 @@ export default { ...@@ -59,7 +63,7 @@ export default {
{{ listEmptyMessage }} {{ listEmptyMessage }}
</li> </li>
<frequent-items-list-item <frequent-items-list-item
v-for="item in items" v-for="item in sanitizedItems"
v-else v-else
:key="item.id" :key="item.id"
:item-id="item.id" :item-id="item.id"
......
import _ from 'underscore'; import _ from 'underscore';
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
import sanitize from 'sanitize-html';
import { FREQUENT_ITEMS, HOUR_IN_MS } from './constants'; import { FREQUENT_ITEMS, HOUR_IN_MS } from './constants';
export const isMobile = () => ['md', 'sm', 'xs'].includes(bp.getBreakpointSize()); export const isMobile = () => ['md', 'sm', 'xs'].includes(bp.getBreakpointSize());
...@@ -43,3 +44,9 @@ export const updateExistingFrequentItem = (frequentItem, item) => { ...@@ -43,3 +44,9 @@ export const updateExistingFrequentItem = (frequentItem, item) => {
lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn, lastAccessedOn: accessedOverHourAgo ? Date.now() : frequentItem.lastAccessedOn,
}; };
}; };
export const sanitizeItem = item => ({
...item,
name: sanitize(item.name.toString(), { allowedTags: [] }),
namespace: sanitize(item.namespace.toString(), { allowedTags: [] }),
});
---
title: Fix xss on frequent groups dropdown
merge_request:
author:
type: security
import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils';
import { isMobile, getTopFrequentItems, updateExistingFrequentItem } from '~/frequent_items/utils'; import {
isMobile,
getTopFrequentItems,
updateExistingFrequentItem,
sanitizeItem,
} from '~/frequent_items/utils';
import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants'; import { HOUR_IN_MS, FREQUENT_ITEMS } from '~/frequent_items/constants';
import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data'; import { mockProject, unsortedFrequentItems, sortedFrequentItems } from './mock_data';
...@@ -92,4 +97,16 @@ describe('Frequent Items utils spec', () => { ...@@ -92,4 +97,16 @@ describe('Frequent Items utils spec', () => {
expect(result.frequency).toBe(mockedProject.frequency); expect(result.frequency).toBe(mockedProject.frequency);
}); });
}); });
describe('sanitizeItem', () => {
it('strips HTML tags for name and namespace', () => {
const input = {
name: '<br><b>test</b>',
namespace: '<br>test',
id: 1,
};
expect(sanitizeItem(input)).toEqual({ name: 'test', namespace: 'test', id: 1 });
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment