Skip to content

Commit b6a9837

Browse files
Bhavesh BhatiCopilot
andcommitted
Harden Azure DevOps Git remote validation
Canonicalize supported HTTPS and SSH remotes before credentials are acquired or sent, and add regression coverage for parser-confusion attacks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 92a843e commit b6a9837

9 files changed

Lines changed: 434 additions & 82 deletions

File tree

azure-devops/azext_devops/dev/common/services.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,14 +30,20 @@
3030
from ._credentials import get_credential
3131
from .git import get_remote_url
3232
from .vsts_git_url_info import VstsGitUrlInfo
33-
from .uri import uri_parse_instance_from_git_uri, is_valid_url
33+
from .uri import (canonicalize_azure_devops_organization_url,
34+
uri_parse_instance_from_git_uri,
35+
is_valid_url)
3436
from .uuid import is_uuid
3537
from .telemetry import vsts_tracking_data, init_telemetry
3638

3739
logger = get_logger(__name__)
3840

3941

4042
def get_connection(organization):
43+
organization = canonicalize_azure_devops_organization_url(organization)
44+
if organization is None:
45+
raise CLIError('The Azure DevOps CLI extension works only with Azure DevOps Services (cloud). '
46+
'It doesn\'t support Azure DevOps Server (on-premises).')
4147
organization = organization.lower()
4248
if organization not in _connection:
4349
credentials = _get_credentials(organization)
@@ -86,6 +92,10 @@ def _get_credentials(organization):
8692
def validate_token_for_instance(organization, credentials):
8793
logger.debug("instance recieved in validate_token_for_instance %s", organization)
8894
organization = uri_parse_instance_from_git_uri(organization)
95+
organization = canonicalize_azure_devops_organization_url(organization)
96+
if organization is None:
97+
logger.debug("Rejected invalid Azure DevOps organization URL during token validation.")
98+
return False
8999
logger.debug("instance processed in validate_token_for_instance %s", organization)
90100
connection = _get_connection(organization, credentials)
91101
core_client = connection.get_client(VSTS_MODULE + 'v5_0.core.core_client.CoreClient')
@@ -166,6 +176,9 @@ def get_token_from_az_login(profile, tenant):
166176

167177

168178
def _get_connection(organization, credentials):
179+
organization = canonicalize_azure_devops_organization_url(organization)
180+
if organization is None:
181+
raise CLIError('Refusing to attach credentials to an invalid Azure DevOps organization URL.')
169182
return Connection(get_base_url(organization), creds=credentials,
170183
user_agent='devOpsCli/{}'.format(VERSION))
171184

@@ -443,9 +456,7 @@ def get_project_id_from_name(organization, project):
443456

444457

445458
def check_organization_in_azure(organization):
446-
startsWith = organization.startswith("https://dev.azure.com/")
447-
endsWith = organization.rstrip("/").endswith(".visualstudio.com")
448-
return startsWith or endsWith
459+
return canonicalize_azure_devops_organization_url(organization) is not None
449460

450461

451462
_connection_data = {}

azure-devops/azext_devops/dev/common/uri.py

Lines changed: 241 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,18 @@
99
from urllib import quote
1010
from urlparse import urlparse
1111

12+
import re
13+
from collections import namedtuple
14+
15+
16+
AzureDevOpsGitRemote = namedtuple(
17+
'AzureDevOpsGitRemote',
18+
['repository_url', 'organization_url', 'host'])
19+
20+
_UNSAFE_URL_CHARACTERS = re.compile(r'[\x00-\x20\x7f\\]')
21+
_SAFE_USERINFO = re.compile(r'^[A-Za-z0-9._-]+$')
22+
_SAFE_HOST_LABEL = re.compile(r'^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$')
23+
1224

1325
def uri_parse(url):
1426
# Special handling for NEW ssh urls which do not start with ssh://
@@ -24,18 +36,79 @@ def uri_quote(query_data):
2436
return quote(query_data)
2537

2638

39+
def parse_azure_devops_git_remote(remote_url):
40+
# Reject characters that URL parsers and HTTP transports may interpret differently.
41+
if not remote_url or _UNSAFE_URL_CHARACTERS.search(remote_url):
42+
return None
43+
44+
lowered_url = remote_url.lower()
45+
if lowered_url.startswith('https://'):
46+
return _parse_azure_devops_https_git_remote(remote_url)
47+
if lowered_url.startswith('ssh://') or '@' in remote_url:
48+
return _parse_azure_devops_ssh_git_remote(remote_url)
49+
return None
50+
51+
52+
def canonicalize_azure_devops_organization_url(url):
53+
if not url or _UNSAFE_URL_CHARACTERS.search(url):
54+
return None
55+
56+
git_remote = parse_azure_devops_git_remote(url)
57+
if git_remote is not None:
58+
return git_remote.organization_url
59+
60+
# Explicit --organization values use a narrower grammar than general service URLs.
61+
parsed_url = _parse_safe_https_url(url, allow_userinfo=False)
62+
if parsed_url is None:
63+
return None
64+
65+
host, path_segments = parsed_url
66+
if host == 'dev.azure.com' or _is_dev_azure_service_host(host):
67+
if len(path_segments) != 1:
68+
return None
69+
return 'https://{host}/{organization}'.format(host=host, organization=path_segments[0])
70+
71+
if _is_visualstudio_host(host) and not path_segments:
72+
return 'https://{host}/'.format(host=host)
73+
return None
74+
75+
76+
def organization_url_from_azure_devops_url(url):
77+
if not url or _UNSAFE_URL_CHARACTERS.search(url):
78+
return None
79+
80+
parsed_url = _parse_safe_https_url(url, allow_userinfo=False)
81+
if parsed_url is None:
82+
return None
83+
84+
host, path_segments = parsed_url
85+
# API and discovery URLs may contain paths after the organization segment.
86+
if host == 'dev.azure.com' or _is_dev_azure_service_host(host):
87+
if not path_segments:
88+
return None
89+
return 'https://{host}/{organization}'.format(host=host, organization=path_segments[0])
90+
91+
if _is_visualstudio_host(host):
92+
return 'https://{host}/'.format(host=host)
93+
return None
94+
95+
96+
def is_azure_devops_host(host):
97+
if not host:
98+
return False
99+
host = host.lower()
100+
return host in ('dev.azure.com', 'ssh.dev.azure.com') or _is_dev_azure_service_host(host) or \
101+
_is_visualstudio_host(host) or host == 'vs-ssh.visualstudio.com'
102+
103+
27104
# Only works for hosted scenario
28105
def uri_parse_instance_from_git_uri(uri):
29-
if "/_git" in uri:
30-
parsed_uri = urlparse(uri)
31-
# old Uri format
32-
if "visualstudio.com" in uri:
33-
return '{uri.scheme}://{uri.netloc}/'.format(uri=parsed_uri)
34-
# new Uri format
35-
if "dev.azure.com" in uri:
36-
org_name = parsed_uri.path.strip("/").split("/")[0]
37-
return parsed_uri.scheme + "://" + parsed_uri.hostname + "/" + org_name
38-
106+
git_remote = parse_azure_devops_git_remote(uri)
107+
if git_remote is not None:
108+
return git_remote.organization_url
109+
organization_url = canonicalize_azure_devops_organization_url(uri)
110+
if organization_url is not None:
111+
return organization_url
39112
return uri
40113

41114

@@ -44,3 +117,161 @@ def is_valid_url(url):
44117
if not parsed_url.scheme or not parsed_url.netloc:
45118
return False
46119
return True
120+
121+
122+
def _parse_azure_devops_https_git_remote(remote_url):
123+
parsed_url = _parse_safe_https_url(remote_url, allow_userinfo=True)
124+
if parsed_url is None:
125+
return None
126+
127+
host, path_segments, userinfo = parsed_url
128+
if host == 'dev.azure.com':
129+
# Modern clone URL: /{organization}/{project}/_git/{repository}.
130+
if len(path_segments) != 4 or path_segments[2].lower() != '_git':
131+
return None
132+
organization = path_segments[0]
133+
if userinfo is not None and userinfo.lower() != organization.lower():
134+
return None
135+
organization_url = 'https://dev.azure.com/{organization}'.format(organization=organization)
136+
elif _is_visualstudio_host(host):
137+
# Legacy URLs can include collection path segments before project/_git/repository.
138+
if len(path_segments) < 3 or path_segments[-2].lower() != '_git':
139+
return None
140+
if userinfo is not None:
141+
return None
142+
organization_url = 'https://{host}/'.format(host=host)
143+
else:
144+
return None
145+
146+
repository_url = 'https://{host}/{path}'.format(
147+
host=host, path='/'.join(path_segments))
148+
return AzureDevOpsGitRemote(repository_url, organization_url, host)
149+
150+
151+
def _parse_azure_devops_ssh_git_remote(remote_url):
152+
parsed_url = None
153+
lowered_url = remote_url.lower()
154+
if lowered_url.startswith('ssh://'):
155+
parsed = urlparse(remote_url)
156+
if parsed.scheme.lower() != 'ssh' or parsed.password or not parsed.username or \
157+
parsed.query or parsed.fragment:
158+
return None
159+
try:
160+
port = parsed.port
161+
except ValueError:
162+
return None
163+
if port not in (None, 22):
164+
return None
165+
parsed_url = (parsed.username, parsed.hostname, parsed.path.strip('/').split('/'))
166+
else:
167+
# Match SCP-style SSH remotes such as git@ssh.dev.azure.com:v3/org/project/repo.
168+
match = re.match(r'^([^@/:]+)@([^@/:]+):(.+)$', remote_url)
169+
if match is None:
170+
return None
171+
# Split the match into SSH username, hostname, and repository path segments.
172+
parsed_url = (match.group(1), match.group(2), match.group(3).strip('/').split('/'))
173+
174+
userinfo, host, path_segments = parsed_url
175+
if not host or not _SAFE_USERINFO.match(userinfo) or any(not segment for segment in path_segments):
176+
return None
177+
178+
host = host.lower()
179+
if path_segments[0].lower() == 'v3':
180+
# Current SSH form: v3/{organization}/{project}/{repository}.
181+
if len(path_segments) != 4:
182+
return None
183+
organization, project, repository = path_segments[1:]
184+
elif len(path_segments) == 3 and path_segments[1].lower() == '_ssh':
185+
# Legacy visualstudio.com URI form: {project}/_ssh/{repository}.
186+
project, repository = path_segments[0], path_segments[2]
187+
organization = userinfo
188+
elif len(path_segments) == 4 and path_segments[2].lower() == '_ssh':
189+
# Legacy dev.azure.com URI form: {organization}/{project}/_ssh/{repository}.
190+
organization, project, repository = path_segments[0], path_segments[1], path_segments[3]
191+
else:
192+
return None
193+
194+
if host == 'ssh.dev.azure.com':
195+
# Convert the modern SSH endpoint to the equivalent canonical HTTPS URL.
196+
if userinfo.lower() != 'git':
197+
return None
198+
https_host = 'dev.azure.com'
199+
organization_url = 'https://dev.azure.com/{organization}'.format(organization=organization)
200+
repository_path = '{organization}/{project}/_git/{repository}'.format(
201+
organization=organization, project=project, repository=repository)
202+
elif host == 'vs-ssh.visualstudio.com':
203+
# The SSH username identifies the organization on the legacy endpoint.
204+
if userinfo.lower() != organization.lower() or not _is_safe_host_label(organization):
205+
return None
206+
https_host = '{organization}.visualstudio.com'.format(organization=organization.lower())
207+
organization_url = 'https://{host}/'.format(host=https_host)
208+
repository_path = '{project}/_git/{repository}'.format(project=project, repository=repository)
209+
else:
210+
return None
211+
212+
repository_url = 'https://{host}/{path}'.format(host=https_host, path=repository_path)
213+
return AzureDevOpsGitRemote(repository_url, organization_url, https_host)
214+
215+
216+
def _parse_safe_https_url(url, allow_userinfo):
217+
if _UNSAFE_URL_CHARACTERS.search(url):
218+
return None
219+
220+
match = re.match(r'^https://([^/?#]*)(/[^?#]*)?/?$', url, re.IGNORECASE)
221+
if match is None:
222+
return None
223+
224+
authority = match.group(1)
225+
# Percent-encoding in the authority can produce different hosts across URL parsers.
226+
if not authority or '%' in authority:
227+
return None
228+
229+
userinfo = None
230+
if '@' in authority:
231+
if not allow_userinfo or authority.count('@') != 1:
232+
return None
233+
userinfo, authority = authority.split('@', 1)
234+
if not userinfo or ':' in userinfo or not _SAFE_USERINFO.match(userinfo):
235+
return None
236+
237+
if ':' in authority:
238+
# Azure DevOps hosted endpoints support only HTTPS on the default port.
239+
host, port = authority.rsplit(':', 1)
240+
if port != '443':
241+
return None
242+
else:
243+
host = authority
244+
245+
host = host.lower()
246+
if not is_azure_devops_host(host):
247+
return None
248+
249+
path = match.group(2) or ''
250+
if '//' in path:
251+
return None
252+
path_segments = [segment for segment in path.strip('/').split('/') if segment]
253+
if allow_userinfo:
254+
return host, path_segments, userinfo
255+
return host, path_segments
256+
257+
258+
def _is_visualstudio_host(host):
259+
suffix = '.visualstudio.com'
260+
if not host.endswith(suffix):
261+
return False
262+
organization = host[:-len(suffix)]
263+
return organization != 'vs-ssh' and _is_safe_host_label(organization)
264+
265+
266+
def _is_dev_azure_service_host(host):
267+
# Azure DevOps APIs can require a service-specific organization endpoint.
268+
# Limit this to one Microsoft-controlled DNS label directly under dev.azure.com.
269+
suffix = '.dev.azure.com'
270+
if not host.endswith(suffix):
271+
return False
272+
service = host[:-len(suffix)]
273+
return service != 'ssh' and _is_safe_host_label(service)
274+
275+
276+
def _is_safe_host_label(label):
277+
return bool(_SAFE_HOST_LABEL.match(label))

0 commit comments

Comments
 (0)