Merge pull request #574 from mq-cloudpak/pranav-2766-asoc-pathtraversal-93015-r1

Code changes to provide sanitised file paths and resolve Asoc Path Traversal vulnerability in v9.3.0.15-r1 (LTS)

Author: Pranav Goyal

cmd/runmqserver/qmgr.go

@@ -18,7 +18,6 @@ package main
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 	"regexp"
 	"strconv"
 	"strings"
@@ -27,6 +26,7 @@ import (
 	containerruntime "github.com/ibm-messaging/mq-container/internal/containerruntime"
 	"github.com/ibm-messaging/mq-container/internal/mqscredact"
 	"github.com/ibm-messaging/mq-container/internal/mqversion"
+	"github.com/ibm-messaging/mq-container/internal/pathutils"
 	"github.com/ibm-messaging/mq-container/internal/ready"
 )
 
@@ -83,7 +83,7 @@ func createQueueManager(name string, devMode bool) (bool, error) {
 
 	// Check if 'qm.ini' configuration file exists for the queue manager
 	// TODO : handle possible race condition - use a file lock?
-	_, err = os.Stat(filepath.Join(dataDir, "qm.ini"))
+	_, err = os.Stat(pathutils.CleanPath(dataDir, "qm.ini"))
 	if err != nil {
 		// If 'qm.ini' is not found - run 'crtmqm' to create a new queue manager
 		args := getCreateQueueManagerArgs(mounts, name, devMode)
@@ -110,7 +110,7 @@ func createQueueManager(name string, devMode bool) (bool, error) {
 // readQMIni reads the qm.ini file and returns it as a byte array
 // This function is specific to comply with the nosec.
 func readQMIni(dataDir string) ([]byte, error) {
-	qmgrDir := filepath.Join(dataDir, "qm.ini")
+	qmgrDir := pathutils.CleanPath(dataDir, "qm.ini")
 	// #nosec G304 - qmgrDir filepath is derived from dspmqinf
 	iniFileBytes, err := os.ReadFile(qmgrDir)
 	if err != nil {
@@ -231,9 +231,9 @@ func isStandbyQueueManager(name string) (bool, error) {
 }
 
 func getQueueManagerDataDir(mounts map[string]string, name string) string {
-	dataDir := filepath.Join("/var/mqm/qmgrs", name)
+	dataDir := pathutils.CleanPath("/var/mqm/qmgrs", name)
 	if _, ok := mounts["/mnt/mqm-data"]; ok {
-		dataDir = filepath.Join("/mnt/mqm-data/qmgrs", name)
+		dataDir = pathutils.CleanPath("/mnt/mqm-data/qmgrs", name)
 	}
 	return dataDir
 }
@@ -314,7 +314,7 @@ func updateQMini(qmname string) error {
 		return err
 	}
 	dataDir := getQueueManagerDataDir(mounts, qmname)
-	qmgrDir := filepath.Join(dataDir, "qm.ini")
+	qmgrDir := pathutils.CleanPath(dataDir, "qm.ini")
 	//read the initial version.
 	// #nosec G304 - qmgrDir filepath is derived from dspmqinf
 	iniFileBytes, err := os.ReadFile(qmgrDir)

internal/filecheck/filecheck.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"path/filepath"
 	"strings"
+
+	"github.com/ibm-messaging/mq-container/internal/pathutils"
 )
 
 // CheckFileSource checks the filename is valid
@@ -29,7 +31,7 @@ func CheckFileSource(fileName string) error {
 
 	prefixes := []string{"bin", "boot", "dev", "lib", "lib64", "proc", "sbin", "sys"}
 	for _, prefix := range prefixes {
-		if strings.HasPrefix(absFile, filepath.Join("/", prefix)) {
+		if strings.HasPrefix(absFile, pathutils.CleanPath("/", prefix)) {
 			return fmt.Errorf("Filename resolves to invalid path '%v'", absFile)
 		}
 	}

internal/pathutils/clean.go

@@ -0,0 +1,39 @@
+/*
+© Copyright IBM Corporation 2024
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package pathutils contains code to provide sanitised file paths
+package pathutils
+
+import (
+	"path"
+	"path/filepath"
+)
+
+// CleanPath returns the result of joining a series of sanitised file paths (preventing directory traversal for each path)
+// If the first path is relative, a relative path is returned
+func CleanPath(paths ...string) string {
+	if len(paths) == 0 {
+		return ""
+	}
+	var combined string
+	if !path.IsAbs(paths[0]) {
+		combined = "./"
+	}
+	for _, part := range paths {
+		combined = filepath.Join(combined, filepath.FromSlash(path.Clean("/"+part)))
+	}
+	return combined
+}

internal/pathutils/clean_test.go

@@ -0,0 +1,45 @@
+/*
+© Copyright IBM Corporation 2024
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package pathutils
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestClean(t *testing.T) {
+
+	tests := []struct {
+		uncleaned string
+		filepath  string
+		cleaned   string
+	}{
+		{"/a/rooted/path", "some.file", "/a/rooted/path/some.file"},
+		{"../../../a/relative/path", "abc.txt", "a/relative/path/abc.txt"},
+		{"a/path" + ".p12", "some.file", "a/path.p12/some.file"},
+		{"/", "bin", "/bin"},
+		{"abc/def", "../../a/relative/path", "abc/def/a/relative/path"},
+	}
+
+	for _, test := range tests {
+		cleaned := CleanPath(test.uncleaned, test.filepath)
+
+		if !strings.EqualFold(cleaned, test.cleaned) {
+			t.Fatalf("file path sanitisation failed. Expected %s but got %s\n", test.cleaned, cleaned)
+		}
+	}
+}

internal/tls/tls.go

@@ -33,6 +33,7 @@ import (
 
 	"github.com/ibm-messaging/mq-container/internal/keystore"
 	"github.com/ibm-messaging/mq-container/internal/mqtemplate"
+	"github.com/ibm-messaging/mq-container/internal/pathutils"
 	"github.com/ibm-messaging/mq-container/pkg/logger"
 )
 
@@ -175,15 +176,15 @@ func generateAllKeystores(keystoreDir string, p12TruststoreRequired bool) (TLSSt
 	}
 
 	// Create the CMS Keystore
-	cmsKeystore.Keystore = keystore.NewCMSKeyStore(filepath.Join(keystoreDir, cmsKeystoreName), cmsKeystore.Password)
+	cmsKeystore.Keystore = keystore.NewCMSKeyStore(pathutils.CleanPath(keystoreDir, cmsKeystoreName), cmsKeystore.Password)
 	err = cmsKeystore.Keystore.Create()
 	if err != nil {
 		return TLSStore{cmsKeystore, p12Truststore}, fmt.Errorf("Failed to create CMS Keystore: %v", err)
 	}
 
 	// Create the PKCS#12 Truststore (if required)
 	if p12TruststoreRequired {
-		p12Truststore.Keystore = keystore.NewPKCS12KeyStore(filepath.Join(keystoreDir, p12TruststoreName), p12Truststore.Password)
+		p12Truststore.Keystore = keystore.NewPKCS12KeyStore(pathutils.CleanPath(keystoreDir, p12TruststoreName), p12Truststore.Password)
 		err = p12Truststore.Keystore.Create()
 		if err != nil {
 			return TLSStore{cmsKeystore, p12Truststore}, fmt.Errorf("Failed to create PKCS#12 Truststore: %v", err)
@@ -205,7 +206,7 @@ func processKeys(tlsStore *TLSStore, keystoreDir string, keyDir string) (string,
 
 		// Process each set of keys - each set should contain files: *.key & *.crt
 		for _, keySet := range keyList {
-			keys, _ := os.ReadDir(filepath.Join(keyDir, keySet.Name()))
+			keys, _ := os.ReadDir(pathutils.CleanPath(keyDir, keySet.Name()))
 
 			// Ensure the label of the set of keys does not match the name of the PKCS#12 Truststore
 			if keySet.Name() == p12TruststoreName[0:len(p12TruststoreName)-len(filepath.Ext(p12TruststoreName))] {
@@ -234,16 +235,17 @@ func processKeys(tlsStore *TLSStore, keystoreDir string, keyDir string) (string,
 			if err != nil {
 				return "", fmt.Errorf("Failed to encode PKCS#12 Keystore %s: %v", keySet.Name()+".p12", err)
 			}
+			keystorePath := pathutils.CleanPath(keystoreDir, keySet.Name()+".p12")
 			// #nosec G306 - this gives permissions to owner/s group only.
-			err = os.WriteFile(filepath.Join(keystoreDir, keySet.Name()+".p12"), file, 0644)
+			err = os.WriteFile(keystorePath, file, 0644)
 			if err != nil {
-				return "", fmt.Errorf("Failed to write PKCS#12 Keystore %s: %v", filepath.Join(keystoreDir, keySet.Name()+".p12"), err)
+				return "", fmt.Errorf("Failed to write PKCS#12 Keystore %s: %v", keystorePath, err)
 			}
 
 			// Import the new PKCS#12 Keystore into the CMS Keystore
-			err = tlsStore.Keystore.Keystore.Import(filepath.Join(keystoreDir, keySet.Name()+".p12"), tlsStore.Keystore.Password)
+			err = tlsStore.Keystore.Keystore.Import(keystorePath, tlsStore.Keystore.Password)
 			if err != nil {
-				return "", fmt.Errorf("Failed to import keys from %s into CMS Keystore: %v", filepath.Join(keystoreDir, keySet.Name()+".p12"), err)
+				return "", fmt.Errorf("Failed to import keys from %s into CMS Keystore: %v", keystorePath, err)
 			}
 
 			// Relabel the certificate in the CMS Keystore
@@ -271,14 +273,15 @@ func processTrustCertificates(tlsStore *TLSStore, trustDir string) error {
 
 		// Process each set of keys
 		for _, trustSet := range trustList {
-			keys, _ := os.ReadDir(filepath.Join(trustDir, trustSet.Name()))
+			keys, _ := os.ReadDir(pathutils.CleanPath(trustDir, trustSet.Name()))
 
 			for _, key := range keys {
 				if strings.HasSuffix(key.Name(), ".crt") {
+					trustSetPath := pathutils.CleanPath(trustDir, trustSet.Name(), key.Name())
 					// #nosec G304 - filename variable is derived from contents of 'trustDir' which is a defined constant
-					file, err := os.ReadFile(filepath.Join(trustDir, trustSet.Name(), key.Name()))
+					file, err := os.ReadFile(trustSetPath)
 					if err != nil {
-						return fmt.Errorf("Failed to read file %s: %v", filepath.Join(trustDir, trustSet.Name(), key.Name()), err)
+						return fmt.Errorf("Failed to read file %s: %v", trustSetPath, err)
 					}
 
 					for string(file) != "" {
@@ -334,15 +337,16 @@ func processPrivateKey(keyDir string, keySetName string, keys []os.DirEntry) (in
 
 	for _, key := range keys {
 
+		privateKeyPath := pathutils.CleanPath(keyDir, keySetName, key.Name())
 		if strings.HasSuffix(key.Name(), ".key") {
 			// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
-			file, err := os.ReadFile(filepath.Join(keyDir, keySetName, key.Name()))
+			file, err := os.ReadFile(privateKeyPath)
 			if err != nil {
-				return nil, "", fmt.Errorf("Failed to read private key %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+				return nil, "", fmt.Errorf("Failed to read private key %s: %v", privateKeyPath, err)
 			}
 			block, _ := pem.Decode(file)
 			if block == nil {
-				return nil, "", fmt.Errorf("Failed to decode private key %s: pem.Decode returned nil", filepath.Join(keyDir, keySetName, key.Name()))
+				return nil, "", fmt.Errorf("Failed to decode private key %s: pem.Decode returned nil", privateKeyPath)
 			}
 
 			// Check if the private key is PKCS1
@@ -351,7 +355,7 @@ func processPrivateKey(keyDir string, keySetName string, keys []os.DirEntry) (in
 				// Check if the private key is PKCS8
 				privateKey, err = x509.ParsePKCS8PrivateKey(block.Bytes)
 				if err != nil {
-					return nil, "", fmt.Errorf("Failed to parse private key %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+					return nil, "", fmt.Errorf("Failed to parse private key %s: %v", privateKeyPath, err)
 				}
 			}
 			keyPrefix = key.Name()[0 : len(key.Name())-len(filepath.Ext(key.Name()))]
@@ -369,19 +373,20 @@ func processCertificates(keyDir string, keySetName, keyPrefix string, keys []os.
 
 	for _, key := range keys {
 
+		keystorePath := pathutils.CleanPath(keyDir, keySetName, key.Name())
 		if strings.HasPrefix(key.Name(), keyPrefix) && strings.HasSuffix(key.Name(), ".crt") {
 			// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
-			file, err := os.ReadFile(filepath.Join(keyDir, keySetName, key.Name()))
+			file, err := os.ReadFile(keystorePath)
 			if err != nil {
-				return nil, nil, fmt.Errorf("Failed to read public certificate %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+				return nil, nil, fmt.Errorf("Failed to read public certificate %s: %v", keystorePath, err)
 			}
 			block, _ := pem.Decode(file)
 			if block == nil {
-				return nil, nil, fmt.Errorf("Failed to decode public certificate %s: pem.Decode returned nil", filepath.Join(keyDir, keySetName, key.Name()))
+				return nil, nil, fmt.Errorf("Failed to decode public certificate %s: pem.Decode returned nil", keystorePath)
 			}
 			publicCertificate, err = x509.ParseCertificate(block.Bytes)
 			if err != nil {
-				return nil, nil, fmt.Errorf("Failed to parse public certificate %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+				return nil, nil, fmt.Errorf("Failed to parse public certificate %s: %v", keystorePath, err)
 			}
 
 			// Add to known certificates for the CMS Keystore
@@ -392,9 +397,9 @@ func processCertificates(keyDir string, keySetName, keyPrefix string, keys []os.
 
 		} else if strings.HasSuffix(key.Name(), ".crt") {
 			// #nosec G304 - filename variable is derived from contents of 'keyDir' which is a defined constant
-			file, err := os.ReadFile(filepath.Join(keyDir, keySetName, key.Name()))
+			file, err := os.ReadFile(keystorePath)
 			if err != nil {
-				return nil, nil, fmt.Errorf("Failed to read CA certificate %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+				return nil, nil, fmt.Errorf("Failed to read CA certificate %s: %v", keystorePath, err)
 			}
 
 			for string(file) != "" {
@@ -420,7 +425,7 @@ func processCertificates(keyDir string, keySetName, keyPrefix string, keys []os.
 
 				certificate, err := x509.ParseCertificate(block.Bytes)
 				if err != nil {
-					return nil, nil, fmt.Errorf("Failed to parse CA certificate %s: %v", filepath.Join(keyDir, keySetName, key.Name()), err)
+					return nil, nil, fmt.Errorf("Failed to parse CA certificate %s: %v", keystorePath, err)
 				}
 				caCertificate = append(caCertificate, certificate)
 			}
@@ -467,7 +472,7 @@ func relabelCertificate(newLabel string, cmsKeystore *KeyStoreData) error {
 // addCertificatesToTruststore adds trust certificates to the PKCS#12 Truststore
 func addCertificatesToTruststore(p12Truststore *KeyStoreData) error {
 
-	temporaryPemFile := filepath.Join("/tmp", "trust.pem")
+	temporaryPemFile := pathutils.CleanPath("/tmp", "trust.pem")
 	_, err := os.Stat(temporaryPemFile)
 	if err == nil {
 		err = os.Remove(temporaryPemFile)
@@ -509,7 +514,7 @@ func addCertificatesToTruststore(p12Truststore *KeyStoreData) error {
 // addCertificatesToCMSKeystore adds trust certificates to the CMS keystore
 func addCertificatesToCMSKeystore(cmsKeystore *KeyStoreData) error {
 
-	temporaryPemFile := filepath.Join("/tmp", "cmsTrust.pem")
+	temporaryPemFile := pathutils.CleanPath("/tmp", "cmsTrust.pem")
 	_, err := os.Stat(temporaryPemFile)
 	if err == nil {
 		err = os.Remove(temporaryPemFile)

internal/tls/tls_web.go

@@ -18,9 +18,9 @@ package tls
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"github.com/ibm-messaging/mq-container/internal/keystore"
+	"github.com/ibm-messaging/mq-container/internal/pathutils"
 )
 
 // webKeystoreDefault is the name of the default web server Keystore
@@ -37,8 +37,8 @@ func ConfigureWebTLS(keyLabel string) error {
 	webConfigDir := "/etc/mqm/web/installations/Installation1/servers/mqweb"
 	tls := "tls.xml"
 
-	tlsConfig := filepath.Join(webConfigDir, tls)
-	newTLSConfig := filepath.Join(webConfigDir, tls+".tpl")
+	tlsConfig := pathutils.CleanPath(webConfigDir, tls)
+	newTLSConfig := pathutils.CleanPath(webConfigDir, tls+".tpl")
 
 	err := os.Remove(tlsConfig)
 	if err != nil {
@@ -60,7 +60,7 @@ func ConfigureWebKeystore(p12Truststore KeyStoreData, webKeystore string) (strin
 	if webKeystore == "" {
 		webKeystore = webKeystoreDefault
 	}
-	webKeystoreFile := filepath.Join(keystoreDirDefault, webKeystore)
+	webKeystoreFile := pathutils.CleanPath(keystoreDirDefault, webKeystore)
 
 	// Check if a new self-signed certificate should be generated
 	genHostName := os.Getenv("MQ_GENERATE_CERTIFICATE_HOSTNAME")

pkg/mqini/mqini.go

@@ -21,10 +21,10 @@ import (
 	"bufio"
 	"errors"
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/ibm-messaging/mq-container/internal/command"
+	"github.com/ibm-messaging/mq-container/internal/pathutils"
 )
 
 // QueueManager describe high-level configuration information for a queue manager
@@ -78,7 +78,7 @@ func GetQueueManager(name string) (*QueueManager, error) {
 // GetErrorLogDirectory returns the directory holding the error logs for the
 // specified queue manager
 func GetErrorLogDirectory(qm *QueueManager) string {
-	return filepath.Join(GetDataDirectory(qm), "errors")
+	return pathutils.CleanPath(GetDataDirectory(qm), "errors")
 }
 
 // GetDataDirectory returns the data directory for the specified queue manager
@@ -87,6 +87,6 @@ func GetDataDirectory(qm *QueueManager) string {
 		// Data path has been set explicitly (e.g. for multi-instance queue manager)
 		return qm.DataPath
 	} else {
-		return filepath.Join(qm.Prefix, "qmgrs", qm.Directory)
+		return pathutils.CleanPath(qm.Prefix, "qmgrs", qm.Directory)
 	}
 }