Merge aa8709359f582ca8b93db5ad95453832e0f6fbf3 into 5cc379fde09edf0753be8d8a871ca5a3003913f7

src/api/api.go

@@ -300,7 +300,10 @@ func (a *Api) RegisterNumber(c *gin.Context) {
 	var req RegisterNumberRequest
 
 	buf := new(bytes.Buffer)
-	buf.ReadFrom(c.Request.Body)
+	if _, err := buf.ReadFrom(c.Request.Body); err != nil {
+		c.JSON(400, Error{Msg: "Couldn't process request - failed to read body."})
+		return
+	}
 	if buf.String() != "" {
 		err := json.Unmarshal(buf.Bytes(), &req)
 		if err != nil {
@@ -352,7 +355,10 @@ func (a *Api) UnregisterNumber(c *gin.Context) {
 	deleteAccount := false
 	deleteLocalData := false
 	buf := new(bytes.Buffer)
-	buf.ReadFrom(c.Request.Body)
+	if _, err := buf.ReadFrom(c.Request.Body); err != nil {
+		c.JSON(400, Error{Msg: "Couldn't process request - failed to read body."})
+		return
+	}
 	if buf.String() != "" {
 		var req UnregisterNumberRequest
 		err := json.Unmarshal(buf.Bytes(), &req)
@@ -432,7 +438,10 @@ func (a *Api) VerifyRegisteredNumber(c *gin.Context) {
 	pin := ""
 	var req VerifyNumberSettings
 	buf := new(bytes.Buffer)
-	buf.ReadFrom(c.Request.Body)
+	if _, err := buf.ReadFrom(c.Request.Body); err != nil {
+		c.JSON(400, Error{Msg: "Couldn't process request - failed to read body."})
+		return
+	}
 	if buf.String() != "" {
 		err := json.Unmarshal(buf.Bytes(), &req)
 		if err != nil {
@@ -583,7 +592,9 @@ func (a *Api) handleSignalReceive(ws *websocket.Conn, number string, stop chan s
 		select {
 		case <-stop:
 			a.signalClient.RemoveReceiveChannel(channelUuid)
-			ws.Close()
+			if err := ws.Close(); err != nil {
+				log.Debug("Error closing websocket: ", err.Error())
+			}
 			return
 		case msg := <-receiveChannel:
 			var data string = string(msg.Params)
@@ -640,7 +651,9 @@ func (a *Api) handleSignalReceive(ws *websocket.Conn, number string, stop chan s
 func wsPong(ws *websocket.Conn, stop chan struct{}) {
 	defer func() {
 		close(stop)
-		ws.Close()
+		if err := ws.Close(); err != nil {
+			log.Debug("Error closing websocket: ", err.Error())
+		}
 	}()
 
 	ws.SetReadLimit(512)
@@ -658,7 +671,9 @@ func (a *Api) wsPing(ws *websocket.Conn, stop chan struct{}) {
 	for {
 		select {
 		case <-stop:
-			ws.Close()
+			if err := ws.Close(); err != nil {
+				log.Debug("Error closing websocket: ", err.Error())
+			}
 			return
 		case <-pingTicker.C:
 			a.wsMutex.Lock()

src/client/attachment.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/gabriel-vasile/mimetype"
 	uuid "github.com/gofrs/uuid"
+	log "github.com/sirupsen/logrus"
 )
 
 type AttachmentEntry struct {
@@ -93,7 +94,7 @@ func (attachmentEntry *AttachmentEntry) storeBase64AsTemporaryFile() error {
 
 	attachmentEntry.DirName = dirNameUuid.String()
 	dirPath := attachmentEntry.attachmentTmpDir + attachmentEntry.DirName
-	if err := os.Mkdir(dirPath, os.ModePerm); err != nil {
+	if err := os.Mkdir(dirPath, 0750); err != nil {
 		return err
 	}
 
@@ -103,29 +104,41 @@ func (attachmentEntry *AttachmentEntry) storeBase64AsTemporaryFile() error {
 	if err != nil {
 		return err
 	}
-	defer f.Close()
 
 	if _, err := f.Write(dec); err != nil {
+		if closeErr := f.Close(); closeErr != nil {
+			log.Error("Couldn't close attachment tmp file: ", closeErr.Error())
+		}
 		attachmentEntry.cleanUp()
 		return err
 	}
 	if err := f.Sync(); err != nil {
+		if closeErr := f.Close(); closeErr != nil {
+			log.Error("Couldn't close attachment tmp file: ", closeErr.Error())
+		}
+		attachmentEntry.cleanUp()
+		return err
+	}
+	if err := f.Close(); err != nil {
 		attachmentEntry.cleanUp()
 		return err
 	}
-	f.Close()
 
 	return nil
 }
 
 func (attachmentEntry *AttachmentEntry) cleanUp() {
 	if strings.Compare(attachmentEntry.FilePath, "") != 0 {
-		os.Remove(attachmentEntry.FilePath)
+		if err := os.Remove(attachmentEntry.FilePath); err != nil {
+			log.Error("Couldn't remove file ", attachmentEntry.FilePath, ": ", err.Error())
+		}
 	}
 
 	if strings.Compare(attachmentEntry.DirName, "") != 0 {
 		dirPath := attachmentEntry.attachmentTmpDir + attachmentEntry.DirName
-		os.Remove(dirPath)
+		if err := os.Remove(dirPath); err != nil {
+			log.Error("Couldn't remove directory ", dirPath, ": ", err.Error())
+		}
 	}
 }
 

src/client/cli.go

@@ -4,11 +4,13 @@ import (
 	"bufio"
 	"bytes"
 	"errors"
-	utils "github.com/bbernhard/signal-cli-rest-api/utils"
+	"fmt"
-	log "github.com/sirupsen/logrus"
 	"os/exec"
 	"strings"
 	"time"
+
+	utils "github.com/bbernhard/signal-cli-rest-api/utils"
+	log "github.com/sirupsen/logrus"
 )
 
 type CliClient struct {
@@ -104,7 +106,12 @@ func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, err
 		cmdTimeout = 120
 	}
 
-	cmd := exec.Command(signalCliBinary, args...)
+	resolvedBinary, err := exec.LookPath(signalCliBinary)
+	if err != nil {
+		return "", fmt.Errorf("signal-cli binary '%s' not found in PATH: %w", signalCliBinary, err)
+	}
+
+	cmd := exec.Command(resolvedBinary, args...) // #nosec G204 -- resolvedBinary is the LookPath-resolved path of a hardcoded binary name ("signal-cli" or "signal-cli-native"), not user-controlled
 	if stdin != "" {
 		cmd.Stdin = strings.NewReader(stdin)
 	}
@@ -161,7 +168,9 @@ func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, err
 		if err != nil {
 			return "", err
 		}
-		cmd.Start()
+		if err := cmd.Start(); err != nil {
+			return "", err
+		}
 		buf := bufio.NewReader(stdout) // Notice that this is not in a loop
 		line, _, _ := buf.ReadLine()
 		return string(line), nil

src/client/client.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -260,7 +259,9 @@ type ListDevicesResponse struct {
 
 func cleanupTmpFiles(paths []string) {
 	for _, path := range paths {
-		os.Remove(path)
+		if err := os.Remove(path); err != nil {
+			log.Error("Couldn't remove tmp file ", path, ": ", err.Error())
+		}
 	}
 }
 
@@ -760,7 +761,7 @@ func (s *SignalClient) About() About {
 		BuildNr:              2,
 		Mode:                 getSignalCliModeString(s.signalCliMode),
 		Version:              utils.GetEnv("BUILD_VERSION", "unset"),
-		Capabilities:         map[string][]string{"v2/send": []string{"quotes", "mentions"}},
+		Capabilities:         map[string][]string{"v2/send": {"quotes", "mentions"}},
 	}
 	return about
 }
@@ -1154,8 +1155,7 @@ func (s *SignalClient) CreateGroup(number string, name string, members []string,
 			Timestamp int64  `json:"timestamp"`
 		}
 		var resp Response
-		json.Unmarshal([]byte(rawData), &resp)
+		if err := json.Unmarshal([]byte(rawData), &resp); err != nil {
-		if err != nil {
 			return "", err
 		}
 		internalGroupId = resp.GroupId
@@ -1758,7 +1758,9 @@ func (s *SignalClient) finishLinkAsync(jsonRpc2Client *JsonRpc2Client, deviceNam
 			return
 		}
 		log.Debug("Linking device result: ", result)
-		s.signalCliApiConfig.Load(s.signalCliApiConfigPath)
+		if err := s.signalCliApiConfig.Load(s.signalCliApiConfigPath); err != nil {
+			log.Error("Couldn't reload signal-cli API config after linking device: ", err.Error())
+		}
 	}()
 }
 
@@ -1851,7 +1853,7 @@ func (s *SignalClient) GetAttachment(attachment string) ([]byte, error) {
 		return []byte{}, &NotFoundError{Description: "No attachment with that name found"}
 	}
 
-	attachmentBytes, err := ioutil.ReadFile(path)
+	attachmentBytes, err := os.ReadFile(path) // #nosec G304 -- path is secured by securejoin.SecureJoin above
 	if err != nil {
 		return []byte{}, &InternalError{Description: "Couldn't read attachment - please try again later"}
 	}
@@ -1878,23 +1880,38 @@ func (s *SignalClient) UpdateProfile(number string, profileName string, base64Av
 			return err
 		}
 
-		avatarTmpPath = s.avatarTmpDir + u.String() + "." + fType.Extension
+		avatarFilename := u.String() + "." + fType.Extension
+		avatarTmpPath = filepath.Join(s.avatarTmpDir, avatarFilename)
 
-		f, err := os.Create(avatarTmpPath)
+		avatarRoot, err := os.OpenRoot(s.avatarTmpDir)
+		if err != nil {
+			return err
+		}
+		defer avatarRoot.Close()
+
+		f, err := avatarRoot.Create(avatarFilename)
 		if err != nil {
 			return err
 		}
-		defer f.Close()
 
 		if _, err := f.Write(avatarBytes); err != nil {
+			if closeErr := f.Close(); closeErr != nil {
+				log.Error("Couldn't close avatar tmp file: ", closeErr.Error())
+			}
 			cleanupTmpFiles([]string{avatarTmpPath})
 			return err
 		}
 		if err := f.Sync(); err != nil {
+			if closeErr := f.Close(); closeErr != nil {
+				log.Error("Couldn't close avatar tmp file: ", closeErr.Error())
+			}
+			cleanupTmpFiles([]string{avatarTmpPath})
+			return err
+		}
+		if err := f.Close(); err != nil {
 			cleanupTmpFiles([]string{avatarTmpPath})
 			return err
 		}
-		f.Close()
 	}
 
 	if s.signalCliMode == JsonRpc {
@@ -2093,23 +2110,38 @@ func (s *SignalClient) UpdateGroup(number string, groupId string, base64Avatar *
 			return err
 		}
 
-		avatarTmpPath = s.avatarTmpDir + u.String() + "." + fType.Extension
+		avatarFilename := u.String() + "." + fType.Extension
+		avatarTmpPath = filepath.Join(s.avatarTmpDir, avatarFilename)
 
-		f, err := os.Create(avatarTmpPath)
+		avatarRoot, err := os.OpenRoot(s.avatarTmpDir)
+		if err != nil {
+			return err
+		}
+		defer avatarRoot.Close()
+
+		f, err := avatarRoot.Create(avatarFilename)
 		if err != nil {
 			return err
 		}
-		defer f.Close()
 
 		if _, err := f.Write(avatarBytes); err != nil {
+			if closeErr := f.Close(); closeErr != nil {
+				log.Error("Couldn't close avatar tmp file: ", closeErr.Error())
+			}
 			cleanupTmpFiles([]string{avatarTmpPath})
 			return err
 		}
 		if err := f.Sync(); err != nil {
+			if closeErr := f.Close(); closeErr != nil {
+				log.Error("Couldn't close avatar tmp file: ", closeErr.Error())
+			}
+			cleanupTmpFiles([]string{avatarTmpPath})
+			return err
+		}
+		if err := f.Close(); err != nil {
 			cleanupTmpFiles([]string{avatarTmpPath})
 			return err
 		}
-		f.Close()
 	}
 
 	if s.signalCliMode == JsonRpc {

src/client/jsonrpc2.go

@@ -224,7 +224,9 @@ func (r *JsonRpc2Client) ReceiveData(number string, receiveWebhookUrl string) {
 		str, err := connbuf.ReadString('\n')
 		if err != nil {
 			log.Error("Lost connection to signal-cli...attempting to reconnect (", err.Error(), ")")
-			r.conn.Close()
+			if err := r.conn.Close(); err != nil {
+				log.Warn("Error closing connection to signal-cli: ", err.Error())
+			}
 			err = r.Dial(r.address, 15)
 			if err != nil {
 				log.Fatal("Unable to reconnect to signal-cli: ", err.Error(), "...aborting")
@@ -236,7 +238,10 @@ func (r *JsonRpc2Client) ReceiveData(number string, receiveWebhookUrl string) {
 		log.Debug("json-rpc received data: ", str)
 
 		var resp1 JsonRpc2ReceivedMessage
-		json.Unmarshal([]byte(str), &resp1)
+		if err := json.Unmarshal([]byte(str), &resp1); err != nil {
+			log.Warn("Couldn't parse received message: ", err.Error())
+			continue
+		}
 		if resp1.Method == "receive" {
 			r.receivedMessagesMutex.Lock()
 			for _, c := range r.receivedMessagesChannels {

src/main.go

@@ -3,9 +3,10 @@ package main
 import (
 	"encoding/json"
 	"flag"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"os"
+	"path/filepath"
 	"plugin"
 	"strconv"
 
@@ -75,7 +76,9 @@ func main() {
 		err := utils.SetLogLevel(logLevel)
 		if err != nil {
 			log.Error("Couldn't set log level to '", logLevel, "'. Falling back to the info log level")
-			utils.SetLogLevel("info")
+			if err := utils.SetLogLevel("info"); err != nil {
+				log.Error("Couldn't set fallback log level to 'info': ", err.Error())
+			}
 		}
 	}
 
@@ -416,9 +419,9 @@ func main() {
 
 		c := cron.New()
 		c.Schedule(schedule, cron.FuncJob(func() {
-			accountsJsonPath := *signalCliConfig + "/data/accounts.json"
+			accountsJsonPath := filepath.Clean(*signalCliConfig + "/data/accounts.json")
 			if _, err := os.Stat(accountsJsonPath); err == nil {
-				signalCliConfigJsonData, err := ioutil.ReadFile(accountsJsonPath)
+				signalCliConfigJsonData, err := os.ReadFile(accountsJsonPath)
 				if err != nil {
 					log.Fatal("AUTO_RECEIVE_SCHEDULE: Couldn't read accounts.json: ", err.Error())
 				}
@@ -452,8 +455,10 @@ func main() {
 					}
 
 					if resp.StatusCode != 200 {
-						jsonResp, err := ioutil.ReadAll(resp.Body)
+						jsonResp, err := io.ReadAll(resp.Body)
-						resp.Body.Close()
+						if closeErr := resp.Body.Close(); closeErr != nil {
+							log.Error("AUTO_RECEIVE_SCHEDULE: Couldn't close response body: ", closeErr.Error())
+						}
 						if err != nil {
 							log.Error("AUTO_RECEIVE_SCHEDULE: Couldn't read json response: ", err.Error())
 							continue
@@ -479,5 +484,7 @@ func main() {
 		c.Start()
 	}
 
-	router.Run()
+	if err := router.Run(); err != nil {
+		log.Fatal("Couldn't start HTTP router: ", err.Error())
+	}
 }

src/scripts/jsonrpc2-helper.go

@@ -2,7 +2,7 @@ package main
 
 import (
 	"fmt"
-	"io/ioutil"
+	"os"
 	"os/exec"
 	"strings"
 
@@ -104,7 +104,7 @@ func main() {
 		signalCliIgnoreAvatars, signalCliIgnoreStickers, tcpPort,
 		supervisorctlProgramName, supervisorctlProgramName)
 
-	err = ioutil.WriteFile(supervisorctlConfigFilename, []byte(supervisorctlConfig), 0644)
+	err = os.WriteFile(supervisorctlConfigFilename, []byte(supervisorctlConfig), 0600)
 	if err != nil {
 		log.Fatal("Couldn't write ", supervisorctlConfigFilename, ": ", err.Error())
 	}

src/utils/api_config.go

@@ -2,9 +2,10 @@ package utils
 
 import (
 	"errors"
-	"gopkg.in/yaml.v2"
-	"io/ioutil"
 	"os"
+	"path/filepath"
+
+	"gopkg.in/yaml.v2"
 )
 
 type SignalCliTrustMode int
@@ -55,9 +56,10 @@ func NewSignalCliApiConfig() *SignalCliApiConfig {
 }
 
 func (c *SignalCliApiConfig) Load(path string) error {
-	c.path = path
+	cleanPath := filepath.Clean(path)
-	if _, err := os.Stat(path); err == nil {
+	c.path = cleanPath
-		data, err := ioutil.ReadFile(path)
+	if _, err := os.Stat(cleanPath); err == nil {
+		data, err := os.ReadFile(cleanPath)
 		if err != nil {
 			return err
 		}
@@ -92,5 +94,5 @@ func (c *SignalCliApiConfig) Persist() error {
 		return err
 	}
 
-	return ioutil.WriteFile(c.path, out, 0644)
+	return os.WriteFile(c.path, out, 0600)
 }

src/utils/config.go

@@ -2,7 +2,8 @@ package utils
 
 import (
 	"errors"
-	"io/ioutil"
+	"os"
+	"path/filepath"
 
 	"gopkg.in/yaml.v2"
 )
@@ -26,7 +27,7 @@ func NewJsonRpc2ClientConfig() *JsonRpc2ClientConfig {
 }
 
 func (c *JsonRpc2ClientConfig) Load(path string) error {
-	data, err := ioutil.ReadFile(path)
+	data, err := os.ReadFile(filepath.Clean(path))
 	if err != nil {
 		return err
 	}
@@ -69,5 +70,5 @@ func (c *JsonRpc2ClientConfig) Persist(path string) error {
 		return err
 	}
 
-	return ioutil.WriteFile(path, out, 0644)
+	return os.WriteFile(filepath.Clean(path), out, 0600)
 }

src/utils/plugin_config.go

@@ -1,7 +1,8 @@
 package utils
 
 import (
-	"io/ioutil"
+	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -25,33 +26,48 @@ type PluginConfigs struct {
 }
 
 func (c *PluginConfigs) Load(baseDirectory string) error {
+	baseDirectory = filepath.Clean(baseDirectory)
+	root, err := os.OpenRoot(baseDirectory)
+	if err != nil {
+		return err
+	}
+	defer root.Close()
 
-	err := filepath.Walk(baseDirectory, func(path string, info os.FileInfo, err error) error {
+	err = filepath.WalkDir(baseDirectory, func(path string, d fs.DirEntry, err error) error {
-		if info.IsDir() {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
 			return nil
 		}
-
 		if filepath.Ext(path) != ".def" {
 			return nil
 		}
 
-		if _, err := os.Stat(path); err == nil {
+		relPath, err := filepath.Rel(baseDirectory, path)
-			data, err := ioutil.ReadFile(path)
+		if err != nil {
+			return err
+		}
+
+		f, err := root.Open(relPath)
+		if err != nil {
+			return err
+		}
+		defer f.Close()
+
+		data, err := io.ReadAll(f)
 		if err != nil {
 			return err
 		}
 
 		var pluginConfig PluginConfig
-			pluginConfig.Version = 1
+		pluginConfig.Version = 1 // default; overridden by yaml if present
-			err = yaml.Unmarshal(data, &pluginConfig)
+		if err = yaml.Unmarshal(data, &pluginConfig); err != nil {
-			if err != nil {
 			return err
 		}
 		pluginConfig.ScriptPath = strings.TrimSuffix(path, filepath.Ext(path)) + ".lua"
 		c.Configs = append(c.Configs, pluginConfig)
-		}
 		return nil
 	})
-
 	return err
 }

src/utils/utils.go

@@ -1,10 +1,10 @@
 package utils
 
 import (
-	"os"
-	"strconv"
 	"errors"
 	log "github.com/sirupsen/logrus"
+	"os"
+	"strconv"
 )
 
 func GetEnv(key string, defaultVal string) string {

src/utils/utils_test.go

@@ -15,7 +15,6 @@ func TestIsPhoneNumber(t *testing.T) {
 	expectEqual(t, res, true)
 }
 
-
 func TestIsPhoneNumberWithSpaces(t *testing.T) {
 	res := IsPhoneNumber("+ 12345678")
 	expectEqual(t, res, true)