Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
Boxiang Sun
gitlab-ce
Commits
fb59eed0
Commit
fb59eed0
authored
May 28, 2019
by
Tiger
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Fix persistent XSS in note objects
parent
516aeaca
Changes
6
Show whitespace changes
Inline
Side-by-side
Showing
6 changed files
with
41 additions
and
3 deletions
+41
-3
changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml
...eleased/security-58856-persistent-xss-in-note-objects.yml
+5
-0
lib/gitlab/import_export/attribute_cleaner.rb
lib/gitlab/import_export/attribute_cleaner.rb
+6
-1
spec/features/projects/import_export/export_file_spec.rb
spec/features/projects/import_export/export_file_spec.rb
+1
-1
spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
+5
-1
spec/lib/gitlab/import_export/project.json
spec/lib/gitlab/import_export/project.json
+4
-0
spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
+20
-0
No files found.
changelogs/unreleased/security-58856-persistent-xss-in-note-objects.yml
0 → 100644
View file @
fb59eed0
---
title
:
Prevent XSS injection in note imports
merge_request
:
author
:
type
:
security
lib/gitlab/import_export/attribute_cleaner.rb
View file @
fb59eed0
...
@@ -4,6 +4,7 @@ module Gitlab
...
@@ -4,6 +4,7 @@ module Gitlab
module
ImportExport
module
ImportExport
class
AttributeCleaner
class
AttributeCleaner
ALLOWED_REFERENCES
=
RelationFactory
::
PROJECT_REFERENCES
+
RelationFactory
::
USER_REFERENCES
+
[
'group_id'
]
ALLOWED_REFERENCES
=
RelationFactory
::
PROJECT_REFERENCES
+
RelationFactory
::
USER_REFERENCES
+
[
'group_id'
]
PROHIBITED_REFERENCES
=
Regexp
.
union
(
/\Acached_markdown_version\Z/
,
/_id\Z/
,
/_html\Z/
).
freeze
def
self
.
clean
(
*
args
)
def
self
.
clean
(
*
args
)
new
(
*
args
).
clean
new
(
*
args
).
clean
...
@@ -24,7 +25,11 @@ module Gitlab
...
@@ -24,7 +25,11 @@ module Gitlab
private
private
def
prohibited_key?
(
key
)
def
prohibited_key?
(
key
)
key
.
end_with?
(
'_id'
)
&&
!
ALLOWED_REFERENCES
.
include?
(
key
)
key
=~
PROHIBITED_REFERENCES
&&
!
permitted_key?
(
key
)
end
def
permitted_key?
(
key
)
ALLOWED_REFERENCES
.
include?
(
key
)
end
end
def
excluded_key?
(
key
)
def
excluded_key?
(
key
)
...
...
spec/features/projects/import_export/export_file_spec.rb
View file @
fb59eed0
...
@@ -12,7 +12,7 @@ describe 'Import/Export - project export integration test', :js do
...
@@ -12,7 +12,7 @@ describe 'Import/Export - project export integration test', :js do
let
(
:export_path
)
{
"
#{
Dir
.
tmpdir
}
/import_file_spec"
}
let
(
:export_path
)
{
"
#{
Dir
.
tmpdir
}
/import_file_spec"
}
let
(
:config_hash
)
{
YAML
.
load_file
(
Gitlab
::
ImportExport
.
config_file
).
deep_stringify_keys
}
let
(
:config_hash
)
{
YAML
.
load_file
(
Gitlab
::
ImportExport
.
config_file
).
deep_stringify_keys
}
let
(
:sensitive_words
)
{
%w[pass secret token key encrypted]
}
let
(
:sensitive_words
)
{
%w[pass secret token key encrypted
html
]
}
let
(
:safe_list
)
do
let
(
:safe_list
)
do
{
{
token:
[
ProjectHook
,
Ci
::
Trigger
,
CommitStatus
],
token:
[
ProjectHook
,
Ci
::
Trigger
,
CommitStatus
],
...
...
spec/lib/gitlab/import_export/attribute_cleaner_spec.rb
View file @
fb59eed0
...
@@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do
...
@@ -18,7 +18,11 @@ describe Gitlab::ImportExport::AttributeCleaner do
'notid'
=>
99
,
'notid'
=>
99
,
'import_source'
=>
'whatever'
,
'import_source'
=>
'whatever'
,
'import_type'
=>
'whatever'
,
'import_type'
=>
'whatever'
,
'non_existent_attr'
=>
'whatever'
'non_existent_attr'
=>
'whatever'
,
'some_html'
=>
'<p>dodgy html</p>'
,
'legit_html'
=>
'<p>legit html</p>'
,
'_html'
=>
'<p>perfectly ordinary html</p>'
,
'cached_markdown_version'
=>
12345
}
}
end
end
...
...
spec/lib/gitlab/import_export/project.json
View file @
fb59eed0
...
@@ -158,6 +158,8 @@
...
@@ -158,6 +158,8 @@
{
{
"id"
:
351
,
"id"
:
351
,
"note"
:
"Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi."
,
"note"
:
"Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi."
,
"note_html"
:
"<p>something else entirely</p>"
,
"cached_markdown_version"
:
917504
,
"noteable_type"
:
"Issue"
,
"noteable_type"
:
"Issue"
,
"author_id"
:
26
,
"author_id"
:
26
,
"created_at"
:
"2016-06-14T15:02:47.770Z"
,
"created_at"
:
"2016-06-14T15:02:47.770Z"
,
...
@@ -2363,6 +2365,8 @@
...
@@ -2363,6 +2365,8 @@
{
{
"id"
:
671
,
"id"
:
671
,
"note"
:
"Sit voluptatibus eveniet architecto quidem."
,
"note"
:
"Sit voluptatibus eveniet architecto quidem."
,
"note_html"
:
"<p>something else entirely</p>"
,
"cached_markdown_version"
:
917504
,
"noteable_type"
:
"MergeRequest"
,
"noteable_type"
:
"MergeRequest"
,
"author_id"
:
26
,
"author_id"
:
26
,
"created_at"
:
"2016-06-14T15:02:56.632Z"
,
"created_at"
:
"2016-06-14T15:02:56.632Z"
,
...
...
spec/lib/gitlab/import_export/project_tree_restorer_spec.rb
View file @
fb59eed0
...
@@ -58,6 +58,26 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
...
@@ -58,6 +58,26 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
expect
(
Milestone
.
find_by_description
(
'test milestone'
).
issues
.
count
).
to
eq
(
2
)
expect
(
Milestone
.
find_by_description
(
'test milestone'
).
issues
.
count
).
to
eq
(
2
)
end
end
context
'when importing a project with cached_markdown_version and note_html'
do
context
'for an Issue'
do
it
'does not import note_html'
do
note_content
=
'Quo reprehenderit aliquam qui dicta impedit cupiditate eligendi'
issue_note
=
Issue
.
find_by
(
description:
'Aliquam enim illo et possimus.'
).
notes
.
select
{
|
n
|
n
.
note
.
match
(
/
#{
note_content
}
/
)}.
first
expect
(
issue_note
.
note_html
).
to
match
(
/
#{
note_content
}
/
)
end
end
context
'for a Merge Request'
do
it
'does not import note_html'
do
note_content
=
'Sit voluptatibus eveniet architecto quidem'
merge_request_note
=
MergeRequest
.
find_by
(
title:
'MR1'
).
notes
.
select
{
|
n
|
n
.
note
.
match
(
/
#{
note_content
}
/
)}.
first
expect
(
merge_request_note
.
note_html
).
to
match
(
/
#{
note_content
}
/
)
end
end
end
it
'creates a valid pipeline note'
do
it
'creates a valid pipeline note'
do
expect
(
Ci
::
Pipeline
.
find_by_sha
(
'sha-notes'
).
notes
).
not_to
be_empty
expect
(
Ci
::
Pipeline
.
find_by_sha
(
'sha-notes'
).
notes
).
not_to
be_empty
end
end
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment