From 65cf02a1d74202dcbc7fb96f945b48d66c585d36 Mon Sep 17 00:00:00 2001
From: Daniel Jiang <jiangd@vmware.com>
Date: Wed, 15 Aug 2018 15:24:16 +0800
Subject: [PATCH] Validate job ID when getting job log

Add validation to job ID in the API to get job log in job service, to
prevent file path traversal attack.

Signed-off-by: Daniel Jiang <jiangd@vmware.com>
---
 src/jobservice/api/handler.go      | 10 ++++++++--
 src/jobservice/api/handler_test.go | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/src/jobservice/api/handler.go b/src/jobservice/api/handler.go
index c631bf398..522d5ba79 100644
--- a/src/jobservice/api/handler.go
+++ b/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
diff --git a/src/jobservice/api/handler_test.go b/src/jobservice/api/handler_test.go
index c89fa7c74..49f2c9225 100644
--- a/src/jobservice/api/handler_test.go
+++ b/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)