Fixing potential bugs / vulnerabilities

src/api/api.go

@@ -304,7 +304,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 {
@@ -356,7 +359,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)
@@ -436,7 +442,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 {
@@ -588,7 +597,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)
@@ -645,7 +656,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)
@@ -663,7 +676,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 {
@@ -90,7 +91,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
 	}
 
@@ -100,29 +101,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"
-	log "github.com/sirupsen/logrus"
+	"fmt"
 	"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"
@@ -258,7 +257,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())
+		}
 	}
 }
 
@@ -743,7 +744,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
 }
@@ -1137,8 +1138,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 != nil {
+		if err := json.Unmarshal([]byte(rawData), &resp); err != nil {
 			return "", err
 		}
 		internalGroupId = resp.GroupId
@@ -1737,7 +1737,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())
+		}
 	}()
 }
 
@@ -1830,7 +1832,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"}
 	}
@@ -1857,23 +1859,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 {
@@ -2072,23 +2089,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)
+
+		avatarRoot, err := os.OpenRoot(s.avatarTmpDir)
+		if err != nil {
+			return err
+		}
+		defer avatarRoot.Close()
 
-		f, err := os.Create(avatarTmpPath)
+		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

@@ -222,7 +222,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")
@@ -234,7 +236,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)
-						resp.Body.Close()
+						jsonResp, err := io.ReadAll(resp.Body)
+						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())
+	}
 }