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
1
Merge Requests
1
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
nexedi
gitlab-ce
Commits
2327897e
Commit
2327897e
authored
Nov 28, 2018
by
Francisco Javier López
Committed by
Cindy Pallares
Nov 28, 2018
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
[master] Fix CRLF issue in UrlValidator
parent
a7ae5c0c
Changes
5
Hide whitespace changes
Inline
Side-by-side
Showing
5 changed files
with
128 additions
and
54 deletions
+128
-54
changelogs/unreleased/security-fj-crlf-injection.yml
changelogs/unreleased/security-fj-crlf-injection.yml
+5
-0
lib/gitlab/url_blocker.rb
lib/gitlab/url_blocker.rb
+14
-5
spec/models/project_spec.rb
spec/models/project_spec.rb
+66
-49
spec/support/shared_contexts/url_shared_context.rb
spec/support/shared_contexts/url_shared_context.rb
+17
-0
spec/validators/url_validator_spec.rb
spec/validators/url_validator_spec.rb
+26
-0
No files found.
changelogs/unreleased/security-fj-crlf-injection.yml
0 → 100644
View file @
2327897e
---
title
:
Fix CRLF vulnerability in Project hooks
merge_request
:
author
:
type
:
security
lib/gitlab/url_blocker.rb
View file @
2327897e
...
...
@@ -10,11 +10,8 @@ module Gitlab
def
validate!
(
url
,
allow_localhost:
false
,
allow_local_network:
true
,
enforce_user:
false
,
ports:
[],
protocols:
[])
return
true
if
url
.
nil?
begin
uri
=
Addressable
::
URI
.
parse
(
url
)
rescue
Addressable
::
URI
::
InvalidURIError
raise
BlockedUrlError
,
"URI is invalid"
end
# Param url can be a string, URI or Addressable::URI
uri
=
parse_url
(
url
)
# Allow imports from the GitLab instance itself but only from the configured ports
return
true
if
internal?
(
uri
)
...
...
@@ -49,6 +46,18 @@ module Gitlab
private
def
parse_url
(
url
)
raise
Addressable
::
URI
::
InvalidURIError
if
multiline?
(
url
)
Addressable
::
URI
.
parse
(
url
)
rescue
Addressable
::
URI
::
InvalidURIError
,
URI
::
InvalidURIError
raise
BlockedUrlError
,
'URI is invalid'
end
def
multiline?
(
url
)
CGI
.
unescape
(
url
.
to_s
)
=~
/\n|\r/
end
def
validate_port!
(
port
,
ports
)
return
if
port
.
blank?
# Only ports under 1024 are restricted
...
...
spec/models/project_spec.rb
View file @
2327897e
...
...
@@ -234,76 +234,93 @@ describe Project do
end
end
it
'does not allow an invalid URI as import_url'
do
project
=
build
(
:project
,
import_url:
'invalid://'
)
describe
'import_url'
do
it
'does not allow an invalid URI as import_url'
do
project
=
build
(
:project
,
import_url:
'invalid://'
)
expect
(
project
).
not_to
be_valid
end
expect
(
project
).
not_to
be_valid
end
it
'does allow a SSH URI as import_url for persisted projects'
do
project
=
create
(
:project
)
project
.
import_url
=
'ssh://test@gitlab.com/project.git'
it
'does allow a SSH URI as import_url for persisted projects'
do
project
=
create
(
:project
)
project
.
import_url
=
'ssh://test@gitlab.com/project.git'
expect
(
project
).
to
be_valid
end
expect
(
project
).
to
be_valid
end
it
'does not allow a SSH URI as import_url for new projects'
do
project
=
build
(
:project
,
import_url:
'ssh://test@gitlab.com/project.git'
)
it
'does not allow a SSH URI as import_url for new projects'
do
project
=
build
(
:project
,
import_url:
'ssh://test@gitlab.com/project.git'
)
expect
(
project
).
not_to
be_valid
end
expect
(
project
).
not_to
be_valid
end
it
'does allow a valid URI as import_url'
do
project
=
build
(
:project
,
import_url:
'http://gitlab.com/project.git'
)
it
'does allow a valid URI as import_url'
do
project
=
build
(
:project
,
import_url:
'http://gitlab.com/project.git'
)
expect
(
project
).
to
be_valid
end
expect
(
project
).
to
be_valid
end
it
'allows an empty URI'
do
project
=
build
(
:project
,
import_url:
''
)
it
'allows an empty URI'
do
project
=
build
(
:project
,
import_url:
''
)
expect
(
project
).
to
be_valid
end
expect
(
project
).
to
be_valid
end
it
'does not produce import data on an empty URI'
do
project
=
build
(
:project
,
import_url:
''
)
it
'does not produce import data on an empty URI'
do
project
=
build
(
:project
,
import_url:
''
)
expect
(
project
.
import_data
).
to
be_nil
end
expect
(
project
.
import_data
).
to
be_nil
end
it
'does not produce import data on an invalid URI'
do
project
=
build
(
:project
,
import_url:
'test://'
)
it
'does not produce import data on an invalid URI'
do
project
=
build
(
:project
,
import_url:
'test://'
)
expect
(
project
.
import_data
).
to
be_nil
end
expect
(
project
.
import_data
).
to
be_nil
end
it
"does not allow import_url pointing to localhost"
do
project
=
build
(
:project
,
import_url:
'http://localhost:9000/t.git'
)
it
"does not allow import_url pointing to localhost"
do
project
=
build
(
:project
,
import_url:
'http://localhost:9000/t.git'
)
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Requests to localhost are not allowed'
)
end
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Requests to localhost are not allowed'
)
end
it
"does not allow import_url with invalid ports for new projects"
do
project
=
build
(
:project
,
import_url:
'http://github.com:25/t.git'
)
it
"does not allow import_url with invalid ports for new projects"
do
project
=
build
(
:project
,
import_url:
'http://github.com:25/t.git'
)
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Only allowed ports are 80, 443'
)
end
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Only allowed ports are 80, 443'
)
end
it
"does not allow import_url with invalid ports for persisted projects"
do
project
=
create
(
:project
)
project
.
import_url
=
'http://github.com:25/t.git'
it
"does not allow import_url with invalid ports for persisted projects"
do
project
=
create
(
:project
)
project
.
import_url
=
'http://github.com:25/t.git'
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Only allowed ports are 22, 80, 443'
)
end
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Only allowed ports are 22, 80, 443'
)
end
it
"does not allow import_url with invalid user"
do
project
=
build
(
:project
,
import_url:
'http://$user:password@github.com/t.git'
)
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Username needs to start with an alphanumeric character'
)
end
it
"does not allow import_url with invalid user"
do
project
=
build
(
:project
,
import_url:
'http://$user:password@github.com/t.git'
)
include_context
'invalid urls'
expect
(
project
).
to
be_invalid
expect
(
project
.
errors
[
:import_url
].
first
).
to
include
(
'Username needs to start with an alphanumeric character'
)
it
'does not allow urls with CR or LF characters'
do
project
=
build
(
:project
)
aggregate_failures
do
urls_with_CRLF
.
each
do
|
url
|
project
.
import_url
=
url
expect
(
project
).
not_to
be_valid
expect
(
project
.
errors
.
full_messages
.
first
).
to
match
(
/is blocked: URI is invalid/
)
end
end
end
end
describe
'project pending deletion'
do
...
...
spec/support/shared_contexts/url_shared_context.rb
0 → 100644
View file @
2327897e
shared_context
'invalid urls'
do
let
(
:urls_with_CRLF
)
do
[
"http://127.0.0.1:333/pa
\r
th"
,
"http://127.0.0.1:333/pa
\n
th"
,
"http://127.0a.0.1:333/pa
\r\n
th"
,
"http://127.0.0.1:333/path?param=foo
\r\n
bar"
,
"http://127.0.0.1:333/path?param=foo
\r
bar"
,
"http://127.0.0.1:333/path?param=foo
\n
bar"
,
"http://127.0.0.1:333/pa%0dth"
,
"http://127.0.0.1:333/pa%0ath"
,
"http://127.0a.0.1:333/pa%0d%0th"
,
"http://127.0.0.1:333/pa%0D%0Ath"
,
"http://127.0.0.1:333/path?param=foo%0Abar"
,
"http://127.0.0.1:333/path?param=foo%0Dbar"
,
"http://127.0.0.1:333/path?param=foo%0D%0Abar"
]
end
end
spec/validators/url_validator_spec.rb
View file @
2327897e
# frozen_string_literal: true
require
'spec_helper'
describe
UrlValidator
do
...
...
@@ -6,6 +8,30 @@ describe UrlValidator do
include_examples
'url validator examples'
,
described_class
::
DEFAULT_PROTOCOLS
describe
'validations'
do
include_context
'invalid urls'
let
(
:validator
)
{
described_class
.
new
(
attributes:
[
:link_url
])
}
it
'returns error when url is nil'
do
expect
(
validator
.
validate_each
(
badge
,
:link_url
,
nil
)).
to
be_nil
expect
(
badge
.
errors
.
first
[
1
]).
to
eq
'must be a valid URL'
end
it
'returns error when url is empty'
do
expect
(
validator
.
validate_each
(
badge
,
:link_url
,
''
)).
to
be_nil
expect
(
badge
.
errors
.
first
[
1
]).
to
eq
'must be a valid URL'
end
it
'does not allow urls with CR or LF characters'
do
aggregate_failures
do
urls_with_CRLF
.
each
do
|
url
|
expect
(
validator
.
validate_each
(
badge
,
:link_url
,
url
)[
0
]).
to
eq
'is blocked: URI is invalid'
end
end
end
end
context
'by default'
do
let
(
:validator
)
{
described_class
.
new
(
attributes:
[
:link_url
])
}
...
...
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