Skip to content

Commit

Permalink
Issue #4232 - Fix CWE-22 Path/Directory Traversal issues
Browse files Browse the repository at this point in the history
Signed-off-by: Oleksandr Mordyk <oleksandr.mordyk1994@outlook.com>
  • Loading branch information
omordyk committed Feb 3, 2025
1 parent 8c9234c commit 66e0d87
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 30 deletions.
7 changes: 5 additions & 2 deletions api/path_publickey.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"io/ioutil"
"os"
"path"
"path/filepath"
"strings"

"github.com/golang/glog"
Expand Down Expand Up @@ -101,6 +102,8 @@ func UploadPublicKey(filename string,

targetPath := config.UserPublicKeyPath()
targetFile := path.Join(targetPath, filename)
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(targetFile)

// Receive the uploaded file content and verify that it is a valid public key or x509 cert. If it's
// valid then save it into the configured PublicKeyPath location from the config. The name of the
Expand All @@ -111,8 +114,8 @@ func UploadPublicKey(filename string,
return errorhandler(NewAPIUserInputError(fmt.Sprintf("provided public key or cert is not valid; error: %v", err), "trusted cert file"))
} else if err := os.MkdirAll(targetPath, 0644); err != nil {
return errorhandler(NewSystemError(fmt.Sprintf("unable to create trusted cert directory %v, error: %v", targetPath, err)))
} else if err := ioutil.WriteFile(targetFile, inBytes, 0644); err != nil {
return errorhandler(NewSystemError(fmt.Sprintf("unable to write uploaded trusted cert file %v, error: %v", targetFile, err)))
} else if err := os.WriteFile(cleanedPath, inBytes, 0644); err != nil {
return errorhandler(NewSystemError(fmt.Sprintf("unable to write uploaded trusted cert file %v, error: %v", cleanedPath, err)))
}
return false
}
Expand Down
10 changes: 5 additions & 5 deletions cli/cliconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bufio"
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -87,10 +86,11 @@ func GetConfig(configFile string) (*HorizonCliConfig, error) {
msgPrinter := i18n.GetMessagePrinter()

cliutils.Verbose(msgPrinter.Sprintf("Reading configuration file: %v", configFile))

fileBytes, err := ioutil.ReadFile(configFile)
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(configFile)
fileBytes, err := os.ReadFile(cleanedPath)
if err != nil {
return nil, fmt.Errorf(msgPrinter.Sprintf("Unable to read config file: %v. %v", configFile, err))
return nil, fmt.Errorf(msgPrinter.Sprintf("Unable to read config file: %v. %v", cleanedPath, err))
}

// Remove /* */ comments
Expand All @@ -99,7 +99,7 @@ func GetConfig(configFile string) (*HorizonCliConfig, error) {

config := HorizonCliConfig{}
if err := json.Unmarshal(newBytes, &config); err != nil {
return nil, fmt.Errorf(msgPrinter.Sprintf("Unable to decode content of config file %v. %v", configFile, err))
return nil, fmt.Errorf(msgPrinter.Sprintf("Unable to decode content of config file %v. %v", cleanedPath, err))
} else {
return &config, nil
}
Expand Down
6 changes: 4 additions & 2 deletions cli/cliutils/cliutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1111,9 +1111,11 @@ func TrustIcpCert(httpClient *http.Client) error {
}

if icpCertPath != "" {
icpCert, err := ioutil.ReadFile(icpCertPath)
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(icpCertPath)
icpCert, err := os.ReadFile(cleanedPath)
if err != nil {
return fmt.Errorf(i18n.GetMessagePrinter().Sprintf("Encountered error reading ICP cert file %v: %v", icpCertPath, err))
return fmt.Errorf(i18n.GetMessagePrinter().Sprintf("Encountered error reading ICP cert file %v: %v", cleanedPath, err))
}
caCertPool.AppendCertsFromPEM(icpCert)
}
Expand Down
12 changes: 8 additions & 4 deletions config/collaborators.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"crypto/x509"
"errors"
"fmt"
"github.com/golang/glog"
"io/ioutil"
"net"
"net/http"
"os"
"path/filepath"
"strings"
"time"

"github.com/golang/glog"
)

// This exists to consolidate construction of clients to collaborating
Expand Down Expand Up @@ -112,11 +114,13 @@ func newHTTPClientFactory(hConfig HorizonConfig) (*HTTPClientFactory, error) {

if mhCertPath != "" {
var err error
mgmtHubBytes, err = ioutil.ReadFile(mhCertPath)
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(mhCertPath)
mgmtHubBytes, err = os.ReadFile(cleanedPath)
if err != nil {
return nil, fmt.Errorf("Failed to read Cert File: %v", mhCertPath)
return nil, fmt.Errorf("Failed to read Cert File: %v", cleanedPath)
}
glog.V(4).Infof("Read Management Hub cert from provided file %v", mhCertPath)
glog.V(4).Infof("Read Management Hub cert from provided file %v", cleanedPath)
}

if hConfig.AgreementBot.CSSSSLCert != "" {
Expand Down
22 changes: 13 additions & 9 deletions container/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"math/big"
"os"
"os/user"
"path"
"path/filepath"
"strconv"
"strings"

"github.com/boltdb/bolt"
"github.com/coreos/go-iptables/iptables"
docker "github.com/fsouza/go-dockerclient"
Expand All @@ -23,14 +32,6 @@ import (
"github.com/open-horizon/anax/resource"
"github.com/open-horizon/anax/worker"
"golang.org/x/sys/unix"
"io"
"io/ioutil"
"math/big"
"os"
"os/user"
"path"
"strconv"
"strings"
)

const LABEL_PREFIX = "openhorizon.anax"
Expand Down Expand Up @@ -1285,7 +1286,10 @@ func (b *ContainerWorker) ResourcesCreate(agreementId string, agreementProtocol

glog.V(5).Infof("Writing raw config to file in %v. Config data: %v", workloadRWStorageDir, string(configureRaw))
// write raw to workloadRWStorageDir
if err := ioutil.WriteFile(path.Join(workloadRWStorageDir, "Configure"), configureRaw, 0644); err != nil {
targetPath := (path.Join(workloadRWStorageDir, "Configure"))
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(targetPath)
if err := os.WriteFile(cleanedPath, configureRaw, 0644); err != nil {
return nil, err
}
} else {
Expand Down
20 changes: 12 additions & 8 deletions css/horizonAuthenticate.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,19 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/open-horizon/anax/exchange"
"github.com/open-horizon/edge-sync-service/core/security"
"github.com/open-horizon/edge-utilities/logger"
"github.com/open-horizon/edge-utilities/logger/log"
"github.com/open-horizon/edge-utilities/logger/trace"
"io/ioutil"
"net"
"net/http"
"os"
"path/filepath"
"strings"
"time"

"github.com/open-horizon/anax/exchange"
"github.com/open-horizon/edge-sync-service/core/security"
"github.com/open-horizon/edge-utilities/logger"
"github.com/open-horizon/edge-utilities/logger/log"
"github.com/open-horizon/edge-utilities/logger/trace"
)

// Set this env var to a value that will be used to identify the http header that contains the user identity, when this
Expand Down Expand Up @@ -516,12 +518,14 @@ func newHTTPClient(certPath string) (*http.Client, error) {

if certPath != "" {
var err error
caBytes, err = ioutil.ReadFile(certPath)
// Clean the path to remove any redundant slashes or path components like ".."
cleanedPath := filepath.Clean(certPath)
caBytes, err = os.ReadFile(cleanedPath)
if err != nil {
return nil, errors.New(fmt.Sprintf("unable to read %v, error %v", certPath, err))
return nil, errors.New(fmt.Sprintf("unable to read %v, error %v", cleanedPath, err))
}
if log.IsLogging(logger.INFO) {
log.Info(cssALS(fmt.Sprintf("read CA cert from provided file %v", certPath)))
log.Info(cssALS(fmt.Sprintf("read CA cert from provided file %v", cleanedPath)))
}
}

Expand Down

0 comments on commit 66e0d87

Please sign in to comment.