WebClient: validate PDF files before rendering

Signed-off-by: Nicola Murino <nicola.murino@gmail.com>

internal/httpd/httpd.go

@@ -159,6 +159,7 @@ const (
 	webClientForgotPwdPathDefault         = "/web/client/forgot-password"
 	webClientResetPwdPathDefault          = "/web/client/reset-password"
 	webClientViewPDFPathDefault           = "/web/client/viewpdf"
+	webClientGetPDFPathDefault            = "/web/client/getpdf"
 	webStaticFilesPathDefault             = "/static"
 	webOpenAPIPathDefault                 = "/openapi"
 	// MaxRestoreSize defines the max size for the loaddata input file
@@ -244,6 +245,7 @@ var (
 	webClientForgotPwdPath         string
 	webClientResetPwdPath          string
 	webClientViewPDFPath           string
+	webClientGetPDFPath            string
 	webStaticFilesPath             string
 	webOpenAPIPath                 string
 	// max upload size for http clients, 1GB by default
@@ -967,6 +969,7 @@ func updateWebClientURLs(baseURL string) {
 	webClientForgotPwdPath = path.Join(baseURL, webClientForgotPwdPathDefault)
 	webClientResetPwdPath = path.Join(baseURL, webClientResetPwdPathDefault)
 	webClientViewPDFPath = path.Join(baseURL, webClientViewPDFPathDefault)
+	webClientGetPDFPath = path.Join(baseURL, webClientGetPDFPathDefault)
 }
 
 func updateWebAdminURLs(baseURL string) {

internal/httpd/httpd_test.go

@@ -174,6 +174,7 @@ const (
 	webClientForgotPwdPath         = "/web/client/forgot-password"
 	webClientResetPwdPath          = "/web/client/reset-password"
 	webClientViewPDFPath           = "/web/client/viewpdf"
+	webClientGetPDFPath            = "/web/client/getpdf"
 	webAdminEventRulesPath         = "/web/admin/eventrules"
 	webAdminEventRulePath          = "/web/admin/eventrule"
 	webAdminEventActionsPath       = "/web/admin/eventactions"
@@ -10941,6 +10942,13 @@ func TestMaxSessions(t *testing.T) {
 	checkResponseCode(t, http.StatusTooManyRequests, rr)
 	assert.Contains(t, rr.Body.String(), "too many open sessions")
 
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=file", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusTooManyRequests, rr)
+	assert.Contains(t, rr.Body.String(), "too many open sessions")
+
 	// test reset password
 	smtpCfg := smtp.Config{
 		Host:          "127.0.0.1",
@@ -12960,16 +12968,105 @@ func TestWebClientViewPDF(t *testing.T) {
 	rr := executeRequest(req)
 	checkResponseCode(t, http.StatusBadRequest, rr)
 
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath, nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+
 	req, err = http.NewRequest(http.MethodGet, webClientViewPDFPath+"?path=test.pdf", nil)
 	assert.NoError(t, err)
 	setJWTCookieForReq(req, webToken)
 	rr = executeRequest(req)
 	checkResponseCode(t, http.StatusOK, rr)
 
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=test.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+	assert.Contains(t, rr.Body.String(), "Unable to get file")
+
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2F", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+	assert.Contains(t, rr.Body.String(), "Invalid file")
+
+	err = os.WriteFile(filepath.Join(user.GetHomeDir(), "test.pdf"), []byte("some text data"), 0666)
+	assert.NoError(t, err)
+
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+	assert.Contains(t, rr.Body.String(), "Invalid PDF file")
+
+	err = createTestFile(filepath.Join(user.GetHomeDir(), "test.pdf"), 1024)
+	assert.NoError(t, err)
+
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusBadRequest, rr)
+	assert.Contains(t, rr.Body.String(), "does not look like a PDF")
+
+	fakePDF := []byte(`%PDF-1.6`)
+	for i := 0; i < 128; i++ {
+		fakePDF = append(fakePDF, []byte(fmt.Sprintf("%d", i))...)
+	}
+	err = os.WriteFile(filepath.Join(user.GetHomeDir(), "test.pdf"), fakePDF, 0666)
+	assert.NoError(t, err)
+
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusOK, rr)
+
+	user.Filters.FilePatterns = []sdk.PatternsFilter{
+		{
+			Path:           "/",
+			DeniedPatterns: []string{"*.pdf"},
+		},
+	}
+	_, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
+	assert.NoError(t, err)
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
+	assert.Contains(t, rr.Body.String(), "Unable to get a reader for the file")
+
+	user.Filters.FilePatterns = []sdk.PatternsFilter{
+		{
+			Path:           "/",
+			DeniedPatterns: []string{"*.txt"},
+		},
+	}
+	user.Filters.DeniedProtocols = []string{common.ProtocolHTTP}
+	_, _, err = httpdtest.UpdateUser(user, http.StatusOK, "")
+	assert.NoError(t, err)
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusForbidden, rr)
+
 	_, err = httpdtest.RemoveUser(user, http.StatusOK)
 	assert.NoError(t, err)
 	err = os.RemoveAll(user.GetHomeDir())
 	assert.NoError(t, err)
+
+	req, err = http.NewRequest(http.MethodGet, webClientGetPDFPath+"?path=%2Ftest.pdf", nil)
+	assert.NoError(t, err)
+	setJWTCookieForReq(req, webToken)
+	rr = executeRequest(req)
+	checkResponseCode(t, http.StatusNotFound, rr)
 }
 
 func TestWebEditFile(t *testing.T) {

internal/httpd/internal_test.go

@@ -2231,6 +2231,13 @@ func TestWebUserInvalidClaims(t *testing.T) {
 	server.handleClientGetShares(rr, req)
 	assert.Equal(t, http.StatusForbidden, rr.Code)
 	assert.Contains(t, rr.Body.String(), "Invalid token claims")
+
+	rr = httptest.NewRecorder()
+	req, _ = http.NewRequest(http.MethodGet, webClientViewPDFPath, nil)
+	req.Header.Set("Cookie", fmt.Sprintf("jwt=%v", token["access_token"]))
+	server.handleClientGetPDF(rr, req)
+	assert.Equal(t, http.StatusForbidden, rr.Code)
+	assert.Contains(t, rr.Body.String(), "Invalid token claims")
 }
 
 func TestInvalidClaims(t *testing.T) {

internal/httpd/server.go

@@ -1427,7 +1427,7 @@ func (s *httpdServer) setupWebClientRoutes() {
 				s.jwtAuthenticatorPartial(tokenAudienceWebClientPartial)).
 				Post(webClientTwoFactorRecoveryPath, s.handleWebClientTwoFactorRecoveryPost)
 		}
-		// share API exposed to external users
+		// share routes exposed to external users
 		s.router.Get(webClientPubSharesPath+"/{id}", s.downloadFromShare)
 		s.router.Get(webClientPubSharesPath+"/{id}/partial", s.handleClientSharePartialDownload)
 		s.router.Get(webClientPubSharesPath+"/{id}/browse", s.handleShareGetFiles)
@@ -1446,6 +1446,7 @@ func (s *httpdServer) setupWebClientRoutes() {
 			router.Get(webClientLogoutPath, s.handleWebClientLogout)
 			router.With(s.checkSecondFactorRequirement, s.refreshCookie).Get(webClientFilesPath, s.handleClientGetFiles)
 			router.With(s.checkSecondFactorRequirement, s.refreshCookie).Get(webClientViewPDFPath, s.handleClientViewPDF)
+			router.With(s.checkSecondFactorRequirement, s.refreshCookie).Get(webClientGetPDFPath, s.handleClientGetPDF)
 			router.With(s.checkSecondFactorRequirement, s.refreshCookie, verifyCSRFHeader).Get(webClientFilePath, getUserFile)
 			router.With(s.checkSecondFactorRequirement, s.checkHTTPUserPerm(sdk.WebClientWriteDisabled), verifyCSRFHeader).
 				Post(webClientFilePath, uploadUserFile)

internal/httpd/webclient.go

@@ -803,9 +803,8 @@ func (s *httpdServer) handleShareGetFiles(w http.ResponseWriter, r *http.Request
 		s.renderSharedFilesPage(w, r, share.GetRelativePath(name), "", share)
 		return
 	}
-	inline := r.URL.Query().Get("inline") != ""
 	dataprovider.UpdateShareLastUse(&share, 1) //nolint:errcheck
-	if status, err := downloadFile(w, r, connection, name, info, inline, &share); err != nil {
+	if status, err := downloadFile(w, r, connection, name, info, false, &share); err != nil {
 		dataprovider.UpdateShareLastUse(&share, -1) //nolint:errcheck
 		if status > 0 {
 			s.renderSharedFilesPage(w, r, path.Dir(share.GetRelativePath(name)), err.Error(), share)
@@ -938,8 +937,7 @@ func (s *httpdServer) handleClientGetFiles(w http.ResponseWriter, r *http.Reques
 		s.renderFilesPage(w, r, name, "", user, len(s.binding.WebClientIntegrations) > 0)
 		return
 	}
-	inline := r.URL.Query().Get("inline") != ""
-	if status, err := downloadFile(w, r, connection, name, info, inline, nil); err != nil && status != 0 {
+	if status, err := downloadFile(w, r, connection, name, info, false, nil); err != nil && status != 0 {
 		if status > 0 {
 			if status == http.StatusRequestedRangeNotSatisfiable {
 				s.renderClientMessagePage(w, r, http.StatusText(status), "", status, err, "")
@@ -1350,9 +1348,80 @@ func (s *httpdServer) handleClientViewPDF(w http.ResponseWriter, r *http.Request
 	name = util.CleanPath(name)
 	data := viewPDFPage{
 		Title:     path.Base(name),
-		URL:       fmt.Sprintf("%v?path=%v&inline=1", webClientFilesPath, url.QueryEscape(name)),
+		URL:       fmt.Sprintf("%s?path=%s&_=%d", webClientGetPDFPath, url.QueryEscape(name), time.Now().UTC().Unix()),
 		StaticURL: webStaticFilesPath,
 		Branding:  s.binding.Branding.WebClient,
 	}
 	renderClientTemplate(w, templateClientViewPDF, data)
 }
+
+func (s *httpdServer) handleClientGetPDF(w http.ResponseWriter, r *http.Request) {
+	r.Body = http.MaxBytesReader(w, r.Body, maxLoginBodySize)
+	claims, err := getTokenClaims(r)
+	if err != nil || claims.Username == "" {
+		s.renderClientForbiddenPage(w, r, "Invalid token claims")
+		return
+	}
+	name := r.URL.Query().Get("path")
+	if name == "" {
+		s.renderClientBadRequestPage(w, r, errors.New("no file specified"))
+		return
+	}
+	name = util.CleanPath(name)
+	user, err := dataprovider.GetUserWithGroupSettings(claims.Username)
+	if err != nil {
+		s.renderClientMessagePage(w, r, "Unable to retrieve your user", "", getRespStatus(err), nil, "")
+		return
+	}
+
+	connID := xid.New().String()
+	protocol := getProtocolFromRequest(r)
+	connectionID := fmt.Sprintf("%v_%v", protocol, connID)
+	if err := checkHTTPClientUser(&user, r, connectionID, false); err != nil {
+		s.renderClientForbiddenPage(w, r, err.Error())
+		return
+	}
+	connection := &Connection{
+		BaseConnection: common.NewBaseConnection(connID, protocol, util.GetHTTPLocalAddress(r),
+			r.RemoteAddr, user),
+		request: r,
+	}
+	if err = common.Connections.Add(connection); err != nil {
+		s.renderClientMessagePage(w, r, "Unable to add connection", "", http.StatusTooManyRequests, err, "")
+		return
+	}
+	defer common.Connections.Remove(connection.GetID())
+
+	info, err := connection.Stat(name, 0)
+	if err != nil {
+		s.renderClientMessagePage(w, r, "Unable to get file", "", getRespStatus(err), err, "")
+		return
+	}
+	if info.IsDir() {
+		s.renderClientMessagePage(w, r, "Invalid file", fmt.Sprintf("%q is not a file", name),
+			http.StatusBadRequest, nil, "")
+		return
+	}
+	connection.User.CheckFsRoot(connection.ID) //nolint:errcheck
+	reader, err := connection.getFileReader(name, 0, r.Method)
+	if err != nil {
+		s.renderClientMessagePage(w, r, fmt.Sprintf("Unable to get a reader for the file %q", name), "",
+			getRespStatus(err), err, "")
+		return
+	}
+	defer reader.Close()
+
+	var b bytes.Buffer
+	_, err = io.CopyN(&b, reader, 128)
+	if err != nil {
+		s.renderClientMessagePage(w, r, "Invalid PDF file", fmt.Sprintf("Unable to validate the file %q as PDF", name),
+			http.StatusBadRequest, nil, "")
+		return
+	}
+	if ctype := http.DetectContentType(b.Bytes()); ctype != "application/pdf" {
+		connection.Log(logger.LevelDebug, "detected %q content type, expected PDF, file %q", ctype, name)
+		s.renderClientBadRequestPage(w, r, fmt.Errorf("the file %q does not look like a PDF", name))
+		return
+	}
+	downloadFile(w, r, connection, name, info, true, nil) //nolint:errcheck
+}