Merge pull request #5628 from reasonerjt/fix-file-traversal-job-lob

Validate job ID when getting job log

src/jobservice/api/handler.go

@@ -7,14 +7,15 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
-
-	"github.com/vmware/harbor/src/jobservice/opm"
+	"os"
+	"strings"
 
 	"github.com/gorilla/mux"
 
 	"github.com/vmware/harbor/src/jobservice/core"
 	"github.com/vmware/harbor/src/jobservice/errs"
 	"github.com/vmware/harbor/src/jobservice/models"
+	"github.com/vmware/harbor/src/jobservice/opm"
 )
 
 //Handler defines approaches to handle the http requests.
@@ -206,6 +207,11 @@ func (dh *DefaultHandler) HandleJobLogReq(w http.ResponseWriter, req *http.Reque
 	vars := mux.Vars(req)
 	jobID := vars["job_id"]
 
+	if strings.Contains(jobID, "..") || strings.ContainsRune(jobID, os.PathSeparator) {
+		dh.handleError(w, http.StatusBadRequest, fmt.Errorf("Invalid Job ID: %s", jobID))
+		return
+	}
+
 	logData, err := dh.controller.GetJobLogData(jobID)
 	if err != nil {
 		code := http.StatusInternalServerError

src/jobservice/api/handler_test.go

@@ -227,6 +227,22 @@ func TestCheckStatus(t *testing.T) {
 	ctx.WG.Wait()
 }
 
+func TestGetJobLogInvalidID(t *testing.T) {
+	exportUISecret(fakeSecret)
+
+	server, port, ctx := createServer()
+	server.Start()
+	<-time.After(200 * time.Millisecond)
+
+	_, err := getReq(fmt.Sprintf("http://localhost:%d/api/v1/jobs/%%2F..%%2Fpasswd/log", port))
+	if err == nil || strings.Contains(err.Error(), "400") {
+		t.Fatalf("Expected 400 error but got: %v", err)
+	}
+
+	server.Stop()
+	ctx.WG.Wait()
+}
+
 func TestGetJobLog(t *testing.T) {
 	exportUISecret(fakeSecret)