Getting logs from unicode logpath via API causes internal server error

[matusf]

What happened:

Hello @viralpoetry and I were fuzzing the k8s (via openapi-fuzzer) and found out following bug. Getting logs from unicode logpath via API causes internal server error. The API endpoint is /logs/{logpath}.

What you expected to happen:

Response with non-500 HTTP status code

How to reproduce it (as minimally and precisely as possible):

We were fuzzing k8s locally via minikube.

1. start minukube

minikube start --driver virtualbox

2. install ca certificates

sudo cp ~/.minikube/ca.pem /usr/local/share/ca-certificates/
sudo update-ca-certificates

3. get k8s IP address

kubectl config view --minify -o jsonpath='{.clusters[0].cluster.server}'

4. add the IP to /etc/hosts as minikubeca
5. get access token and export it as token

kubectl create clusterrolebinding permissive-binding \
  --clusterrole=cluster-admin \
  --user=admin \
  --user=kubelet \
  --group=system:serviceaccounts

kubectl create serviceaccount openapifuzz
kubectl get serviceaccounts openapifuzz -o json | jq -r .secrets[].name
kubectl get secret openapifuzz-token-sn9qm -o json | jq -r .data.token
 export token=<retrieved-token>

6. send request

curl -X GET -H "authorization: bearer $token" https://minikubeca:8443/logs/%F3%95%BE%A2%F4%8C%97%BC%E8%91%A0%F1%B4%82%AA%F1%8F%B6%9B%F1%BE%B2%A4%F1%84%89%94%F3%B5%BC%93%F1%A6%AE%BB%F3%81%9A%9A%F1%A6%BF%B8%F3%B7%93%83%F4%88%B4%9B%F2%AF%85%A1%F3%88%87%82%F3%A5%B7%8F%F0%BD%B4%80%E2%BC%87%F0%A9%96%A7%F3%89%B7%A8%F0%AD%B4%B5%F1%B8%AE%BA%F3%9E%A3%8A%F3%96%B7%91%F2%BE%83%A7%F3%92%83%BB%F2%BB%96%9E%F3%8F%92%AB%F2%91%AD%92%F3%86%B8%87%F0%9F%94%86%F3%94%8F%BD%F2%8F%8A%B2%F1%A2%B5%9B%F2%96%B6%81%E6%B0%9C%F1%9A%A4%85%F1%A9%9E%93%F2%9B%8D%A8%F3%A0%9D%9E%F1%9C%95%B5%F2%87%AD%B3%F2%92%85%9E%F3%B9%9A%9F%F0%B4%81%91%F1%8F%AC%A0%F0%A7%AF%9D%F4%8B%80%B9%F3%91%BA%84%E4%AA%89%F3%AC%99%9A%F2%A8%B4%AF%F1%8F%94%BC%F2%9B%BC%9A%F3%A1%B7%AF%F2%B3%B4%B0%F2%83%99%97%F2%86%A7%B0%F0%AD%8D%8D%F3%B0%B4%AC%F3%B8%B0%AA%F1%83%A9%90%F1%92%A9%B0%F3%88%AD%86%F4%80%B3%B4

500 Internal Server Error

Environment:

• Kubernetes version (use kubectl version):

clientVersion:
  buildDate: "2021-04-08T16:31:21Z"
  compiler: gc
  gitCommit: cb303e613a121a29364f75cc67d3d580833a7479
  gitTreeState: clean
  gitVersion: v1.21.0
  goVersion: go1.16.1
  major: "1"
  minor: "21"
  platform: linux/amd64
serverVersion:
  buildDate: "2021-01-13T13:20:00Z"
  compiler: gc
  gitCommit: faecb196815e248d3ecfb03c680a4507229c2a56
  gitTreeState: clean
  gitVersion: v1.20.2
  goVersion: go1.15.5
  major: "1"
  minor: "20"
  platform: linux/amd64

• Cloud provider or hardware configuration: minikube with virtual-box driver
• OS (e.g: cat /etc/os-release): Ubuntu 18.04.5 LTS (bionic)
• Kernel (e.g. uname -a): Linux pine 4.15.0-140-generic Refactor controller manager. #144-Ubuntu SMP Fri Mar 19 14:12:35 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
• Install tools: downloaded precompiled binary
• minikube version: v1.19.0 (commit: 15cede53bdc5fe242228853e737333b09d4336b5)

[matusf]

/sig api-machinery

[Haleygo]

/assign
I will look into it.

[fedebongio]

thank you @Haleygo , please keep us posted!
/triage accepted

[sftim]

For clarity @matusf: are you saying that the fuzzed URL is valid or invalid UTF-8 (URL-encoded)?

[matusf]

It's valid UTF-8

[Haleygo]

Actually, according to my test,this bug comes from http.ServeFile which replies to the request with the contents of the named file or directory.
When giving too long(On Linux: The maximum length for a file name is 255 bytes. The maximum combined length of both the file name and path name is 4096 bytes.) filename or pathname to http.ServeFile,it will return code 500(file name too long) rather than 404(no such file or directory). And when filename is not that long, unicode path work correctly too.

Here is my simple test below:

package main

import (
"net/http"
"os"
)

func main() {
http.HandleFunc("/openValidFile", openValidFile)
http.HandleFunc("/openFileWithLongName", openFileWithLongName)
http.HandleFunc("/openFileWithShortName", openFileWithShortName)

http.ListenAndServe(":8080", nil)
}

// response 200
func openValidFile(res http.ResponseWriter, req *http.Request) {
path, _ := os.Getwd()

fileName := path + "/files/readme.txt"
http.ServeFile(res, req, fileName)
}

// response 500 ,file name too long
func openFileWithLongName(res http.ResponseWriter, req *http.Request) {
path, _ := os.Getwd()

fileName := path + "/files/%F3%95%BE%A2%F4%8C%97%BC%E8%91%A0%F1%B4%82%AA%F1%8F%B6%9B%F1%BE%B2%A4%F1%84%89%94%F3%B5%BC%93%F1%A6%AE%BB%F3%81%9A%9A%F1%A6%BF%B8%F3%B7%93%83%F4%88%B4%9B%F2%AF%85%A1%F3%88%87%82%F3%A5%B7%8F%F0%BD%B4%80%E2%BC%87%F0%A9%96%A7%F3%89%B7%A8%F0%AD%B4%B5%F1%B8%AE%BA%F3%9E%A3%8A%F3%96%B7%91%F2%BE%83%A7%F3%92%83%BB%F2%BB%96%9E%F3%8F%92%AB%F2%91%AD%92%F3%86%B8%87%F0%9F%94%86%F3%94%8F%BD%F2%8F%8A%B2%F1%A2%B5%9B%F2%96%B6%81%E6%B0%9C%F1%9A%A4%85%F1%A9%9E%93%F2%9B%8D%A8%F3%A0%9D%9E%F1%9C%95%B5%F2%87%AD%B3%F2%92%85%9E%F3%B9%9A%9F%F0%B4%81%91%F1%8F%AC%A0%F0%A7%AF%9D%F4%8B%80%B9%F3%91%BA%84%E4%AA%89%F3%AC%99%9A%F2%A8%B4%AF%F1%8F%94%BC%F2%9B%BC%9A%F3%A1%B7%AF%F2%B3%B4%B0%F2%83%99%97%F2%86%A7%B0%F0%AD%8D%8D%F3%B0%B4%AC%F3%B8%B0%AA%F1%83%A9%90%F1%92%A9%B0%F3%88%AD%86%F4%80%B3%B4"
http.ServeFile(res, req, fileName)
}

// response is 404 ,no such file or directory
func openFileWithShortName(res http.ResponseWriter, req *http.Request) {
path, _ := os.Getwd()

fileName := path + "/files/%F3%95%BE%A2%F4%8C%97%BC%E8%91%A0%F1%B4%82%AA%F1%8F%B6%9B%F1%BE%B2%A4%F1%84%89%94%F3%B5%BC%93%F1%A6%AE%BB%F3%81%9A%9A%F1%A6%BF%B8%F3%B7%93%83%F4%88%B4%9B%F2%AF%85%A1%F3%88%87%82"
http.ServeFile(res, req, fileName)
}

And since /logs is the only place use http.ServeFile, there is no other api has the same problem.

So I will add a length check before calling http.ServeFile to solve this.

Does that sound good to you? @matusf

[matusf]

Maybe 400 - bad request would be more suitable response code. But I'm not a maintainer so it's not up to me to decide.

[k8s-triage-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue or PR with /reopen
• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.