From 8efbd30d9518b2ead6a9e613e565a3b4a4b85d73 Mon Sep 17 00:00:00 2001
From: Fabian Seidl <fabian.b.seidl@stud.h-da.de>
Date: Thu, 24 Mar 2022 14:36:55 +0000
Subject: [PATCH] 169 Handling of some vulnerablities after mono

See merge request danet/gosdn!259
---
 cli/cmd/init.go                              |  5 +++-
 cli/cmd/prompt.go                            |  6 ++++-
 controller/config/config.go                  |  6 ++++-
 controller/northbound/server/core.go         |  9 ++++++-
 controller/nucleus/principalNetworkDomain.go | 27 +++++++++++++++++++-
 csbi/run.go                                  |  7 ++++-
 6 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/cli/cmd/init.go b/cli/cmd/init.go
index 85d88fbc9..833af65bd 100644
--- a/cli/cmd/init.go
+++ b/cli/cmd/init.go
@@ -73,7 +73,10 @@ func init() {
 	rootCmd.AddCommand(initCmd)
 
 	initCmd.Flags().StringVar(&controllerAPIEndpoint, "controller", "gosdn-develop.apps.ocp.fbi.h-da.de:55055", "address of the controller")
-	viper.BindPFlag("controllerAPIEndpoint", initCmd.Flags().Lookup("controller"))
+	err := viper.BindPFlag("controllerAPIEndpoint", initCmd.Flags().Lookup("controller"))
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Could not bind controllerAPIEndpoint:", err)
+	}
 
 	// Set controller flag as required (possibly not?)
 	//if err := initCmd.MarkFlagRequired("controller"); err != nil {
diff --git a/cli/cmd/prompt.go b/cli/cmd/prompt.go
index 656435f76..aa4c341f6 100644
--- a/cli/cmd/prompt.go
+++ b/cli/cmd/prompt.go
@@ -86,7 +86,11 @@ func executeFunc(s string) {
 		return
 	}
 	rootCmd.SetArgs(strings.Fields(s))
-	rootCmd.Execute()
+	err := rootCmd.Execute()
+
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "Could not execute:", err)
+	}
 }
 
 func flagVisitor(f *pflag.Flag) {
diff --git a/controller/config/config.go b/controller/config/config.go
index eac7d0e79..dab434d61 100644
--- a/controller/config/config.go
+++ b/controller/config/config.go
@@ -35,7 +35,11 @@ var LogLevel logrus.Level
 
 // Init gets called on module import
 func Init() {
-	InitializeConfig()
+	err := InitializeConfig()
+	if err != nil {
+		log.Error("failed initialization of module import", err)
+	}
+
 }
 
 // InitializeConfig loads the configuration
diff --git a/controller/northbound/server/core.go b/controller/northbound/server/core.go
index c88be01c0..ef7c61732 100644
--- a/controller/northbound/server/core.go
+++ b/controller/northbound/server/core.go
@@ -99,7 +99,14 @@ func (s core) DeletePnd(ctx context.Context, request *pb.DeletePndRequest) (*pb.
 	if err != nil {
 		return nil, handleRPCError(labels, err)
 	}
-	pndc.Delete(pndID)
+
+	err = pndc.Delete(pndID)
+	if err != nil {
+		return &pb.DeletePndResponse{
+			Timestamp: time.Now().UnixNano(),
+			Status:    pb.Status_STATUS_ERROR,
+		}, err
+	}
 
 	return &pb.DeletePndResponse{
 		Timestamp: time.Now().UnixNano(),
diff --git a/controller/nucleus/principalNetworkDomain.go b/controller/nucleus/principalNetworkDomain.go
index 5ecd5e57b..a4dbf0354 100644
--- a/controller/nucleus/principalNetworkDomain.go
+++ b/controller/nucleus/principalNetworkDomain.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 
 	"code.fbi.h-da.de/danet/gosdn/controller/metrics"
@@ -620,6 +621,15 @@ func saveGenericClientStreamToFile(t GenericGrpcClient, filename string, id uuid
 	folderName := viper.GetString("plugin-folder")
 	path := filepath.Join(folderName, id.String(), filename)
 
+	// clean path to prevent attackers to get access to to directories elsewhere on the system
+	path = filepath.Clean(path)
+	if !strings.HasPrefix(path, folderName) {
+		return uuid.Nil, &errors.ErrInvalidParameters{
+			Func:  saveGenericClientStreamToFile,
+			Param: path,
+		}
+	}
+
 	// create the directory hierarchy based on the path
 	if err := os.MkdirAll(filepath.Dir(path), 0770); err != nil {
 		return uuid.Nil, err
@@ -629,7 +639,12 @@ func saveGenericClientStreamToFile(t GenericGrpcClient, filename string, id uuid
 	if err != nil {
 		return uuid.Nil, err
 	}
-	defer f.Close()
+
+	defer func() {
+		if err := f.Close(); err != nil {
+			log.Error("error closing file: ", err)
+		}
+	}()
 
 	// receive byte stream
 	for {
@@ -639,11 +654,21 @@ func saveGenericClientStreamToFile(t GenericGrpcClient, filename string, id uuid
 				break
 			}
 			t.CloseSend()
+			closeErr := t.CloseSend()
+			if closeErr != nil {
+				return uuid.Nil, closeErr
+			}
+
 			return uuid.Nil, err
 		}
 		n, err := f.Write(payload.Chunk)
 		if err != nil {
 			t.CloseSend()
+			closeErr := t.CloseSend()
+			if closeErr != nil {
+				return uuid.Nil, closeErr
+			}
+
 			return uuid.Nil, err
 		}
 		log.WithField("n", n).Trace("wrote bytes")
diff --git a/csbi/run.go b/csbi/run.go
index 1e81a75fe..96373dd02 100644
--- a/csbi/run.go
+++ b/csbi/run.go
@@ -56,7 +56,12 @@ func Run(bindAddr string) {
 		signal.Reset(os.Interrupt)
 		ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 		defer cancel()
-		stopHttpServer(ctx)
+		err := stopHttpServer(ctx)
+
+		if err != nil {
+			log.WithFields(log.Fields{}).Info(err)
+		}
+
 		o.Shutdown(ctx)
 	}()
 
-- 
GitLab