Skip to content

Commit 5dbd918

Browse files
authored
[AKS] az aks check-acr: Fix command injection via unquoted tempfile path in subprocess call (#33155)
1 parent b7ac2f9 commit 5dbd918

2 files changed

Lines changed: 5 additions & 38 deletions

File tree

src/azure-cli/azure/cli/command_modules/acs/custom.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2161,8 +2161,8 @@ def aks_check_acr(cmd, client, resource_group_name, name, acr, node_name=None):
21612161
# Get kubectl minor version
21622162
kubectl_minor_version = -1
21632163
try:
2164-
cmd = f"kubectl version -o json --kubeconfig {browse_path}"
2165-
output = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE)
2164+
kubectl_cmd = ["kubectl", "version", "-o", "json", "--kubeconfig", browse_path]
2165+
output = subprocess.Popen(kubectl_cmd, stdout=subprocess.PIPE)
21662166
jsonS, _ = output.communicate()
21672167
kubectl_version = json.loads(jsonS)
21682168
# Remove any non-numeric characters like + from minor version

src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py

Lines changed: 3 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7489,16 +7489,13 @@ def test_aks_create_network_cidr(self, resource_group, resource_group_location):
74897489
@live_only()
74907490
@AllowLargeResponse()
74917491
@AKSCustomResourceGroupPreparer(random_name_length=17, name_prefix='clitest', location='westus2')
7492-
@AKSCustomRoleBasedServicePrincipalPreparer()
7493-
def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp_name, sp_password):
7492+
def test_aks_create_attach_acr(self, resource_group, resource_group_location):
74947493
aks_name = self.create_random_name('cliakstest', 16)
74957494
acr_name = self.create_random_name('cliaksacr', 16)
74967495
self.kwargs.update({
74977496
'name': aks_name,
74987497
'resource_group': resource_group,
74997498
'ssh_key_value': self.generate_ssh_keys(),
7500-
'service_principal': sp_name,
7501-
'client_secret': sp_password,
75027499
'acr_name': acr_name
75037500
})
75047501

@@ -7510,18 +7507,9 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp
75107507

75117508
# create
75127509
create_cmd = 'aks create --resource-group={resource_group} --name={name} ' \
7513-
'--service-principal={service_principal} --client-secret={client_secret} ' \
75147510
'--ssh-key-value={ssh_key_value} --attach-acr={acr_name}'
75157511
self.cmd(create_cmd, checks=[
75167512
self.check('provisioningState', 'Succeeded'),
7517-
self.check('servicePrincipalProfile.clientId', sp_name)
7518-
])
7519-
7520-
# add Mariner node pool
7521-
node_pool_cmd = 'aks nodepool add --resource-group={resource_group} --cluster-name={name} ' \
7522-
'-n marinerpool --os-sku mariner'
7523-
self.cmd(node_pool_cmd, checks=[
7524-
self.check('provisioningState', 'Succeeded')
75257513
])
75267514

75277515
# install kubectl
@@ -7554,7 +7542,7 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp
75547542
finally:
75557543
os.close(fd)
75567544
# get node name
7557-
k_get_node_cmd = ["kubectl", "get", "node", "-l", "kubernetes.azure.com/os-sku=Ubuntu", "-o", "name", "--kubeconfig", browse_path]
7545+
k_get_node_cmd = ["kubectl", "get", "node", "-o", "name", "--kubeconfig", browse_path]
75587546
k_get_node_output = subprocess.check_output(
75597547
k_get_node_cmd,
75607548
universal_newlines=True,
@@ -7567,35 +7555,14 @@ def test_aks_create_attach_acr(self, resource_group, resource_group_location, sp
75677555
"node_name": node_name,
75687556
}
75697557
)
7570-
# check acr from Ubuntu node
7558+
# check acr
75717559
check_cmd = "aks check-acr -n {name} -g {resource_group} --acr {acr_name}.azurecr.io --node-name {node_name}"
75727560
self.cmd(
75737561
check_cmd,
75747562
checks=[
75757563
StringContainCheck("Your cluster can pull images from {}.azurecr.io!".format(acr_name)),
75767564
],
75777565
)
7578-
# check acr from Mariner node
7579-
k_get_mariner_node_cmd = ["kubectl", "get", "node", "-l", "kubernetes.azure.com/os-sku=Mariner", "-o", "name", "--kubeconfig", browse_path]
7580-
k_get_node_output = subprocess.check_output(
7581-
k_get_mariner_node_cmd,
7582-
universal_newlines=True,
7583-
stderr=subprocess.STDOUT,
7584-
)
7585-
mariner_node_names = k_get_node_output.split("\n")
7586-
mariner_node_name = mariner_node_names[0].strip().strip("node/").strip()
7587-
self.kwargs.update(
7588-
{
7589-
"mariner_node_name": mariner_node_name,
7590-
}
7591-
)
7592-
check_mariner_cmd = "aks check-acr -n {name} -g {resource_group} --acr {acr_name}.azurecr.io --node-name {mariner_node_name}"
7593-
self.cmd(
7594-
check_mariner_cmd,
7595-
checks=[
7596-
StringContainCheck("Your cluster can pull images from {}.azurecr.io!".format(acr_name)),
7597-
],
7598-
)
75997566
# clean up test hook file even if test failed
76007567
finally:
76017568
if os.path.exists(hook_file_path):

0 commit comments

Comments
 (0)