kubernetes: Port parsing can overflow (TOB-K8S-015: Overflows when using strconv.Atoi and downcasting the result)
This issue was reported in the Kubernetes Security Audit Report
Description The strconv.Atoi function parses an int - a machine dependent integer type, which, for 64-bit targets will be int64. There are places throughout the codebase where the result returned from strconv.Atoi is later converted to a smaller type: int16 or int32. This may overflow with a certain input. An example of the issue has been included in Figure 1.
v, err := strconv.Atoi(options.DiskMBpsReadWrite)
diskMBpsReadWrite = int32(v)
Figure 13.1: pkg/cloudprovider/providers/azure/azure_managedDiskController.go:105
Additionally, there are many code paths that parse ports, and do so differently and in a manner lacking checks for a proper port range. An example of this has been identified within kubectl when handling port values.
Kubectl has the ability to expose particular Pod ports through the use of kubectl expose. This command uses the function updatePodPorts, which uses strconv.Atoi to parse a string into an integer, then downcasts it to an int32 (Figure 2).
// updatePodContainers updates PodSpec.Containers.Ports with passed parameters.
func updatePodPorts(params map[string]string, podSpec *v1.PodSpec) (err error) {
port := -1
hostPort := -1
if len(params["port"]) > 0 {
port, err = strconv.Atoi(params["port"]) // <-- this should parse port as strconv.ParseUint(params["port"], 10, 16)
if err != nil {
return err
}
}
// (...)
// Don't include the port if it was not specified.
if len(params["port"]) > 0 {
podSpec.Containers[0].Ports = []v1.ContainerPort{
{
ContainerPort: int32(port), // <-- this should later just be uint16(port)
},
}
Figure 13.2: Relevant snippet of the updatePodPorts function.
This error has been operationalized into a crash within kubectl when overflowing provided ports. Starting with a standard deployment with no services, we can observe the expected behavior (Figure 3).
root@k8s-1:~# cat nginx.yml
apiVersion: apps/v1 # for versions before 1.9.0 use apps/v1beta2
kind: Deployment
metadata:
name: nginx-deployment
spec:
selector:
matchLabels:
app: nginx
replicas: 1 # tells deployment to run 2 pods matching the template
template:
metadata:
labels:
app: nginx
spec:
containers:
- name: nginx
image: nginx:1.7.9
ports:
- containerPort: 80
root@k8s-1:~# kubectl create -f nginx.yml
deployment.apps/nginx-deployment created
root@k8s-1:~# kubectl get pods
NAME READY STATUS RESTARTS AGE
nginx-deployment-76bf4969df-nskjh 1/1 Running 0 2m14s
root@k8s-1:~# kubectl get services
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
kubernetes ClusterIP 10.233.0.1 <none> 443/TCP 30m
Figure 13.3: The deployment spec with service and Pod status.
To trigger the overflow, we can now update the deployment through the kubectl expose command with an overflown port, overflowing from 4294967377 to 81 (Figure 4).
root@k8s-1:/home/vagrant# kubectl expose deployment nginx-deployment --port 4294967377 --target-port 80
service/nginx-deployment exposed
Figure 13.4: Overflowing the port parameter.
We are now able to observe this overflown port when listing the services with kubectl get services (Figure 5). We are also able to access the service on the overflown port (Figure 6).
root@k8s-1:/home/vagrant# kubectl get services
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
kubernetes ClusterIP 10.233.0.1 <none> 443/TCP 42m
nginx-deployment ClusterIP 10.233.25.138 <none> 81/TCP 2s
Figure 13.5: The overflown port got exposed.
root@k8s-1:/home/vagrant# curl 10.233.25.138:81
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
...
Figure 13.6: The result of curling the overflown service port.
Furthering this issue, we are able to also overflow the target port. After deleting the service, we can attempt to overflow the target port as well, which will result in a panic in kubectl (Figure 7 and 8).
root@k8s-1:/home/vagrant# kubectl delete service nginx-deployment
service "nginx-deployment" deleted
root@k8s-1:/home/vagrant# kubectl get services
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
kubernetes ClusterIP 10.233.0.1 <none> 443/TCP 45m
Figure 13.7: The deletion of the deployment.
root@k8s-1:/home/vagrant# kubectl expose deployment nginx-deployment --port 4294967377 --target-port 4294967376
E0402 09:25:31.888983 3625 intstr.go:61] value: 4294967376 overflows int32
goroutine 1 [running]:
runtime/debug.Stack(0xc000e54eb8, 0xc4f1e9b8, 0xa3ce32e2a3d43b34)
/usr/local/go/src/runtime/debug/stack.go:24 +0xa7
k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/util/intstr.FromInt(0x100000050, 0xa, 0x100000050, 0x0, 0x0)
...
service/nginx-deployment exposed
Figure 13.8: The panic in kubectl when overflowing the target port.
Despite the panic from kubectl (visible in Figure 8), the service is still exposed (Figure 9) and accessible (Figure 10).
root@k8s-1:/home/vagrant# kubectl get services
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
kubernetes ClusterIP 10.233.0.1 <none> 443/TCP 46m
nginx-deployment ClusterIP 10.233.59.190 <none> 81/TCP 35s
Figure 13.9: The service is exposed despite the kubectl panic and overflow.
root@k8s-1:/home/vagrant# curl 10.233.59.190:81
<!DOCTYPE html>
<html>
<head>
<title>Welcome to nginx!</title>
...
Figure 13.10: The service is also accessible after overflow.
Exploit Scenario A value is parsed from a configuration file with Atoi, resulting in an integer. It is then downcasted to a lower precision value, resulting in a potential overflow or underflow which is not raised as an error or panic.
Recommendation Short term, when parsing strings into fixed-width integer types, use strconv.ParseInt or strconv.ParseUint with appropriate bitSize argument instead of strconv.Atoi.
Long term, ensure the validity of data and types. Parse and validate values with common functions. For example the ParsePort (cmd/kubeadm/app/util/endpoint.go:117) utility function parses and validates TCP port values, but it is not well used across the codebase.
Anything else we need to know?:
See #81146 for current status of all issues created from these findings.
The vendor gave this issue an ID of TOB-K8S-015 and it was finding 13 of the report.
The vendor considers this issue Medium Severity.
To view the original finding, begin on page 42 of the Kubernetes Security Review Report
Environment:
- Kubernetes version: 1.13.4
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 26 (11 by maintainers)
I run a little variant analysis of this bug class on kubernetes and all their staging packages and I thought you might find the result useful:
“call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/apimachinery/revision-2019-August-10–09-40-29/src/pkg/util/intstr/intstr.go@63:46-63:48” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/csi-translation/revision-2019-August-10–09-41-28/src/plugins/aws_ebs.go@132:31-132:39” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/csi-translation/revision-2019-August-10–09-41-28/src/plugins/gce_pd.go@296:31-296:37” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/cmd/portforward/portforward.go@181:85-181:91” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/cmd/portforward/portforward.go@187:12-187:18” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/cmd/rollingupdate/rolling_updater.go@204:19-204:35” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@106:19-106:23” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@194:19-194:23” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@282:19-282:23” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@817:19-817:23” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@887:26-887:29” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/run.go@891:52-891:59” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/service.go@174:21-174:24” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/service_basic.go@99:16-99:19” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/kubectl/revision-2019-August-10–09-41-42/src/pkg/generate/versioned/service_basic.go@114:15-114:18” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/legacy-cloud-providers/revision-2019-August-10–09-42-05/src/azure/azure_loadbalancer.go@545:16-545:17” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/staging/legacy-cloud-providers/revision-2019-August-10–09-42-05/src/azure/azure_managedDiskController.go@118:30-118:30” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/controller/deployment/util/deployment_util.go@394:15-394:22” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/controller/deployment/util/deployment_util.go@855:9-855:15” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/proxy/ipvs/proxier.go@1710:20-1710:26” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/proxy/ipvs/proxier.go@1753:20-1753:26” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/volume/fc/fc_util.go@171:34-171:36” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/volume/iscsi/iscsi.go@253:27-253:29” “call to Atoi”,“Atoi() used in combination of a int wrapper could cause overflow - “,”/home/nico/Semmle/Projects/Kubernetes/kubernetes/revision-2019-August-06–15-29-05/src/pkg/volume/iscsi/iscsi.go@659:26-659:28”
Hey ya! I found an interesting case in apimachinery https://github.com/kubernetes/apimachinery/blob/master/pkg/api/resource/suffix.go#L190
I have re-titled this. I started the ball rolling. This is a great bug for someone who isn’t super familiar with the codebase and wants to explore a bit.
The only hard part is re-vendoring k/utils and that is not hard.
My first PR was merged to change the utils for this, now I will work on migrating the logic that uses ParseInt to ParsePort in utils. Hopefully should get to that this weekend.