cwtch.im
/
cwtch
16
20
Fork 15
Code Issues 10 Pull Requests 1 Releases 163 Wiki Activity

Properly remove bad profile dir #435

Merged
dan merged 2 commits from import_export into master 2022-03-09 23:58:37 +00:00
Conversation 3 Commits 2 Files Changed 2 +15 -2
2 changed files with 15 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/app.go
View File

@ -129,7 +129,7 @@ func (app *application) AddPeerPlugin(onion string, pluginID plugins.PluginID) {
func (app *application) ImportProfile(exportedCwtchFile string, password string) (peer.CwtchPeer, error) { func (app *application) ImportProfile(exportedCwtchFile string, password string) (peer.CwtchPeer, error) {
profileDirectory := path.Join(app.directory, "profiles") profileDirectory := path.Join(app.directory, "profiles")
profile, err := peer.ImportProfile(exportedCwtchFile, profileDirectory, password) profile, err := peer.ImportProfile(exportedCwtchFile, profileDirectory, password)
if err == nil { if profile != nil || err == nil {
app.installProfile(profile) app.installProfile(profile)
} }
return profile, err return profile, err

15
peer/storage.go
View File

@ -5,6 +5,7 @@ import (
"compress/gzip" "compress/gzip"
"crypto/rand" "crypto/rand"
"database/sql" "database/sql"
"encoding/hex"
"errors" "errors"
"fmt" "fmt"
"git.openprivacy.ca/openprivacy/log" "git.openprivacy.ca/openprivacy/log"
@ -191,7 +192,8 @@ func ImportProfile(exportedCwtchFile string, profilesDir string, password string
return profile, err return profile, err
} }
// Otherwise purge // Otherwise purge
os.RemoveAll(filepath.Join(profilesDir, profileDir)) log.Errorf("error importing profile: %v. removing %s", err, profileDir)
dan marked this conversation as resolved
dan commented 2022-03-09 23:47:57 +00:00
Review
Copy Link

is it possible a malicious profile tarball could be constructed with a "id" / directory named '..' or something that on import and failure it trigers a deletion of the profiles directory or worse?

is it possible a malicious profile tarball could be constructed with a "id" / directory named '..' or something that on import and failure it trigers a deletion of the profiles directory or worse?
os.RemoveAll(profileDir)
return nil, err return nil, err
} }
return nil, err return nil, err
@ -237,6 +239,11 @@ func checkCwtchProfileBackupFile(srcFile string) (string, error) {
dir := parts[0] dir := parts[0]
profileFileType := parts[1] profileFileType := parts[1]
_, hexErr := hex.DecodeString(dir)
if dir == "." || dir == ".." || len(dir) !=32 || hexErr != nil {
return "", errors.New("invalid profile name")
}
if profileName == "" { if profileName == "" {
profileName = dir profileName = dir
} }
@ -292,6 +299,12 @@ func importCwtchProfileBackupFile(srcFile string, profilesDir string) error {
} }
dir := parts[0] dir := parts[0]
base := parts[1] base := parts[1]
_, hexErr := hex.DecodeString(dir)
if dir == "." || dir == ".." || len(dir) != 32 || hexErr != nil {
return errors.New("invalid profile name")
}
if profileName == "" { if profileName == "" {
profileName = dir profileName = dir
} }