From 0651bca94bf28489ab176b91542fafa7bacad6a6 Mon Sep 17 00:00:00 2001 From: zetles <90190656+Zetles96@users.noreply.github.com> Date: Thu, 21 May 2026 14:48:06 +0200 Subject: [PATCH 1/2] fix: Fixed potential errors / vulnerabilities found by gosec --- src/api/api.go | 27 +++++++++++---- src/client/attachment.go | 23 ++++++++++--- src/client/cli.go | 17 +++++++--- src/client/client.go | 60 ++++++++++++++++++++++++++-------- src/client/jsonrpc2.go | 9 +++-- src/main.go | 21 ++++++++---- src/scripts/jsonrpc2-helper.go | 4 +-- src/utils/api_config.go | 14 ++++---- src/utils/config.go | 7 ++-- src/utils/plugin_config.go | 56 +++++++++++++++++++++---------- 10 files changed, 172 insertions(+), 66 deletions(-) diff --git a/src/api/api.go b/src/api/api.go index 3ec81a26..f9a4ffe2 100644 --- a/src/api/api.go +++ b/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() diff --git a/src/client/attachment.go b/src/client/attachment.go index a51d9022..7765ba3d 100644 --- a/src/client/attachment.go +++ b/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()) + } } } diff --git a/src/client/cli.go b/src/client/cli.go index 23d11e72..879c4782 100644 --- a/src/client/cli.go +++ b/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 diff --git a/src/client/client.go b/src/client/client.go index 5dc59857..11eb0108 100644 --- a/src/client/client.go +++ b/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()) + } } } @@ -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 { diff --git a/src/client/jsonrpc2.go b/src/client/jsonrpc2.go index ef006336..9a49554f 100644 --- a/src/client/jsonrpc2.go +++ b/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 { diff --git a/src/main.go b/src/main.go index 24504965..a18a49ff 100644 --- a/src/main.go +++ b/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()) + } } } @@ -404,9 +407,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()) } @@ -440,8 +443,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 @@ -467,5 +472,7 @@ func main() { c.Start() } - router.Run() + if err := router.Run(); err != nil { + log.Fatal("Couldn't start HTTP router: ", err.Error()) + } } diff --git a/src/scripts/jsonrpc2-helper.go b/src/scripts/jsonrpc2-helper.go index 96a4c34f..325b04c7 100644 --- a/src/scripts/jsonrpc2-helper.go +++ b/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()) } diff --git a/src/utils/api_config.go b/src/utils/api_config.go index 85abeb6f..2e638d79 100644 --- a/src/utils/api_config.go +++ b/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 - if _, err := os.Stat(path); err == nil { - data, err := ioutil.ReadFile(path) + cleanPath := filepath.Clean(path) + c.path = cleanPath + 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) } diff --git a/src/utils/config.go b/src/utils/config.go index 9163b2c1..c08362ef 100644 --- a/src/utils/config.go +++ b/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) } diff --git a/src/utils/plugin_config.go b/src/utils/plugin_config.go index 53492169..40633750 100644 --- a/src/utils/plugin_config.go +++ b/src/utils/plugin_config.go @@ -1,11 +1,13 @@ package utils import ( - "gopkg.in/yaml.v2" - "io/ioutil" + "io" + "io/fs" "os" "path/filepath" "strings" + + "gopkg.in/yaml.v2" ) type PluginConfig struct { @@ -23,9 +25,20 @@ 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 { - if info.IsDir() { + err = filepath.WalkDir(baseDirectory, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { return nil } @@ -33,20 +46,29 @@ func (c *PluginConfigs) Load(baseDirectory string) error { return nil } - if _, err := os.Stat(path); err == nil { - data, err := ioutil.ReadFile(path) - if err != nil { - return err - } - - var pluginConfig PluginConfig - err = yaml.Unmarshal(data, &pluginConfig) - if err != nil { - return err - } - pluginConfig.ScriptPath = strings.TrimSuffix(path, filepath.Ext(path)) + ".lua" - c.Configs = append(c.Configs, pluginConfig) + relPath, err := filepath.Rel(baseDirectory, 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 + if err = yaml.Unmarshal(data, &pluginConfig); err != nil { + return err + } + pluginConfig.ScriptPath = strings.TrimSuffix(path, filepath.Ext(path)) + ".lua" + c.Configs = append(c.Configs, pluginConfig) + return nil }) From 6c774054fd95a5c6e362791f0c155fc45e088c3a Mon Sep 17 00:00:00 2001 From: zetles <90190656+Zetles96@users.noreply.github.com> Date: Thu, 21 May 2026 14:49:22 +0200 Subject: [PATCH 2/2] chore: Formattet the project --- src/client/client.go | 2 +- src/plugin_loader.go | 28 ++++++++++++++-------------- src/utils/utils.go | 4 ++-- src/utils/utils_test.go | 13 ++++++------- 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/client/client.go b/src/client/client.go index 11eb0108..daddf8e1 100644 --- a/src/client/client.go +++ b/src/client/client.go @@ -744,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 } diff --git a/src/plugin_loader.go b/src/plugin_loader.go index fa114475..0487fa9d 100644 --- a/src/plugin_loader.go +++ b/src/plugin_loader.go @@ -1,27 +1,27 @@ package main import ( - "github.com/yuin/gopher-lua" + "github.com/bbernhard/signal-cli-rest-api/api" + "github.com/bbernhard/signal-cli-rest-api/utils" "github.com/cjoudrey/gluahttp" - "layeh.com/gopher-luar" - luajson "layeh.com/gopher-json" "github.com/gin-gonic/gin" - "io" log "github.com/sirupsen/logrus" - "github.com/bbernhard/signal-cli-rest-api/utils" - "github.com/bbernhard/signal-cli-rest-api/api" - "strings" + "github.com/yuin/gopher-lua" + "io" + luajson "layeh.com/gopher-json" + "layeh.com/gopher-luar" "net/http" + "strings" ) type PluginInputData struct { - Params map[string]string + Params map[string]string QueryParams map[string]string - Payload string + Payload string } type PluginOutputData struct { - payload string + payload string httpStatusCode int } @@ -50,13 +50,13 @@ func execPlugin(c *gin.Context, pluginConfig utils.PluginConfig) { } pluginInputData := &PluginInputData{ - Params: make(map[string]string), + Params: make(map[string]string), QueryParams: make(map[string]string), - Payload: string(jsonData), + Payload: string(jsonData), } pluginOutputData := &PluginOutputData{ - payload: "", + payload: "", httpStatusCode: 200, } @@ -98,5 +98,5 @@ func (p plugHandler) ExecutePlugin(pluginConfig utils.PluginConfig) gin.HandlerF return gin.HandlerFunc(fn) } -//exported +// exported var PluginHandler plugHandler diff --git a/src/utils/utils.go b/src/utils/utils.go index 882309d0..b1b33562 100644 --- a/src/utils/utils.go +++ b/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 { diff --git a/src/utils/utils_test.go b/src/utils/utils_test.go index 63160ff3..a8354499 100644 --- a/src/utils/utils_test.go +++ b/src/utils/utils_test.go @@ -11,32 +11,31 @@ func expectEqual(t *testing.T, in1 bool, in2 bool) { } func TestIsPhoneNumber(t *testing.T) { - res := IsPhoneNumber("+12345678") + res := IsPhoneNumber("+12345678") expectEqual(t, res, true) } - func TestIsPhoneNumberWithSpaces(t *testing.T) { - res := IsPhoneNumber("+ 12345678") + res := IsPhoneNumber("+ 12345678") expectEqual(t, res, true) } func TestIsPhoneNumberWithSpaces1(t *testing.T) { - res := IsPhoneNumber("+ 1234 5678") + res := IsPhoneNumber("+ 1234 5678") expectEqual(t, res, true) } func TestIsPhoneNumberWithInvalidCharacters(t *testing.T) { - res := IsPhoneNumber("+123456x") + res := IsPhoneNumber("+123456x") expectEqual(t, res, false) } func TestIsPhoneNumberWithMissingPrefix(t *testing.T) { - res := IsPhoneNumber("123456x") + res := IsPhoneNumber("123456x") expectEqual(t, res, false) } func TestIsPhoneNumberWithInvalidCharactersAndSpaces(t *testing.T) { - res := IsPhoneNumber("+12345 6x") + res := IsPhoneNumber("+12345 6x") expectEqual(t, res, false) }