From 1a43bdaca5e81f71e94eb721d886c31a60ddb3c1 Mon Sep 17 00:00:00 2001 From: ziggy Date: Fri, 10 Jul 2026 18:07:57 +0300 Subject: [PATCH 1/2] Validate message body ranges and harden receive JSON handling --- src/client/cli.go | 24 ++++++++--- src/client/cli_test.go | 20 +++++++++ src/client/client.go | 80 ++++++++++++++++++++++++++++++++---- src/client/receive_test.go | 50 ++++++++++++++++++++++ src/utils/textstyleparser.go | 8 ++-- 5 files changed, 163 insertions(+), 19 deletions(-) create mode 100644 src/client/cli_test.go create mode 100644 src/client/receive_test.go diff --git a/src/client/cli.go b/src/client/cli.go index 23d11e72..0c82bda2 100644 --- a/src/client/cli.go +++ b/src/client/cli.go @@ -4,11 +4,12 @@ import ( "bufio" "bytes" "errors" - utils "github.com/bbernhard/signal-cli-rest-api/utils" - 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 { @@ -49,6 +50,19 @@ func stripInfoAndWarnMessages(input string) (string, string, string) { return output, infoMessages, warnMessages } +func classifySignalCliOutput(stdout string, stderr string) (string, string, string) { + stdout = strings.TrimRight(stdout, "\r\n") + output, infoMessages, warnMessages := stripInfoAndWarnMessages(stdout) + stderr = strings.TrimSpace(stderr) + if stderr != "" { + if warnMessages != "" { + warnMessages += "\n" + } + warnMessages += stderr + } + return output, infoMessages, warnMessages +} + func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, error) { containerId, err := getContainerId() @@ -132,17 +146,15 @@ func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, err return "", errors.New("process killed as timeout reached") case err := <-done: if err != nil { - combinedOutput := stdoutBuffer.String() + stderrBuffer.String() log.Debug("signal-cli output (stdout): ", stdoutBuffer.String()) log.Debug("signal-cli output (stderr): ", stderrBuffer.String()) - return "", errors.New(combinedOutput) + return "", errors.New(strings.TrimSpace(stdoutBuffer.String() + stderrBuffer.String())) } } - combinedOutput := stdoutBuffer.String() + stderrBuffer.String() log.Debug("signal-cli output (stdout): ", stdoutBuffer.String()) log.Debug("signal-cli output (stderr): ", stderrBuffer.String()) - strippedOutput, infoMessages, warnMessages := stripInfoAndWarnMessages(combinedOutput) + strippedOutput, infoMessages, warnMessages := classifySignalCliOutput(stdoutBuffer.String(), stderrBuffer.String()) for _, line := range strings.Split(infoMessages, "\n") { if line != "" { log.Info(line) diff --git a/src/client/cli_test.go b/src/client/cli_test.go new file mode 100644 index 00000000..05e6cd11 --- /dev/null +++ b/src/client/cli_test.go @@ -0,0 +1,20 @@ +package client + +import ( + "strings" + "testing" +) + +func TestClassifySignalCliOutputKeepsStderrOutOfResponse(t *testing.T) { + stdout := "{\"account\":\"+380000000001\"}\n" + stderr := "WARN IncomingMessageHandler - Invalid content! reason\njava.lang.Throwable\n\tat example" + + output, _, warnings := classifySignalCliOutput(stdout, stderr) + + if output != strings.TrimSpace(stdout) { + t.Fatalf("got output %q, wanted %q", output, strings.TrimSpace(stdout)) + } + if !strings.Contains(warnings, "Invalid content! reason") || !strings.Contains(warnings, "java.lang.Throwable") { + t.Fatalf("warnings did not preserve stderr: %q", warnings) + } +} diff --git a/src/client/client.go b/src/client/client.go index 92008db7..023e3ea6 100644 --- a/src/client/client.go +++ b/src/client/client.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -497,6 +498,18 @@ func (s *SignalClient) send(signalCliSendRequest ds.SignalCliSendRequest) (*Send textstyleParser := utils.NewTextstyleParser(signalCliSendRequest.Message) signalCliSendRequest.Message, signalCliTextFormatStrings = textstyleParser.Parse() } + if err := validateBodyRanges(signalCliSendRequest.Message, + signalCliSendRequest.Mentions, + signalCliTextFormatStrings); err != nil { + return nil, err + } + if signalCliSendRequest.QuoteMessage != nil { + if err := validateMentionRanges(*signalCliSendRequest.QuoteMessage, signalCliSendRequest.QuoteMentions); err != nil { + return nil, fmt.Errorf("invalid quote mention: %w", err) + } + } else if len(signalCliSendRequest.QuoteMentions) > 0 { + return nil, errors.New("quote mentions require a quote message") + } var groupId string = "" if signalCliSendRequest.RecipientType == ds.Group { @@ -1053,20 +1066,69 @@ func (s *SignalClient) Receive(number string, timeout int64, ignoreAttachments b return "", err } - out = strings.Trim(out, "\n") - lines := strings.Split(out, "\n") + return marshalJsonStream(out) + } +} - jsonStr := "[" - for i, line := range lines { - jsonStr += line - if i != (len(lines) - 1) { - jsonStr += "," +func marshalJsonStream(output string) (string, error) { + decoder := json.NewDecoder(strings.NewReader(output)) + messages := make([]json.RawMessage, 0) + for { + var message json.RawMessage + if err := decoder.Decode(&message); err != nil { + if errors.Is(err, io.EOF) { + break } + return "", fmt.Errorf("invalid JSON from signal-cli receive: %w", err) + } + messages = append(messages, message) + } + + result, err := json.Marshal(messages) + if err != nil { + return "", fmt.Errorf("failed to marshal signal-cli receive response: %w", err) + } + return string(result), nil +} + +func validateBodyRanges(message string, mentions []ds.MessageMention, textStyles []string) error { + if err := validateMentionRanges(message, mentions); err != nil { + return fmt.Errorf("invalid mention: %w", err) + } + + bodyLength := int64(utils.UTF16StringLength(message)) + for i, textStyle := range textStyles { + parts := strings.SplitN(textStyle, ":", 3) + if len(parts) != 3 { + return fmt.Errorf("invalid text style at index %d", i) + } + start, err := strconv.ParseInt(parts[0], 10, 64) + if err != nil { + return fmt.Errorf("invalid text style start at index %d: %w", i, err) + } + length, err := strconv.ParseInt(parts[1], 10, 64) + if err != nil { + return fmt.Errorf("invalid text style length at index %d: %w", i, err) + } + if !bodyRangeWithinBounds(start, length, bodyLength) { + return fmt.Errorf("text style at index %d is outside the final UTF-16 message length %d", i, bodyLength) } - jsonStr += "]" + } + return nil +} - return jsonStr, nil +func validateMentionRanges(message string, mentions []ds.MessageMention) error { + bodyLength := int64(utils.UTF16StringLength(message)) + for i, mention := range mentions { + if !bodyRangeWithinBounds(mention.Start, mention.Length, bodyLength) { + return fmt.Errorf("mention at index %d is outside the final UTF-16 message length %d", i, bodyLength) + } } + return nil +} + +func bodyRangeWithinBounds(start int64, length int64, bodyLength int64) bool { + return start >= 0 && length >= 0 && start <= bodyLength && length <= bodyLength-start } func (s *SignalClient) GetReceiveChannel() (chan JsonRpc2ReceivedMessage, string, error) { diff --git a/src/client/receive_test.go b/src/client/receive_test.go new file mode 100644 index 00000000..b9cc239a --- /dev/null +++ b/src/client/receive_test.go @@ -0,0 +1,50 @@ +package client + +import ( + "encoding/json" + "testing" + + ds "github.com/bbernhard/signal-cli-rest-api/datastructs" + utils "github.com/bbernhard/signal-cli-rest-api/utils" +) + +func TestMarshalJsonStream(t *testing.T) { + output, err := marshalJsonStream("{\"account\":\"one\"}\n{\"account\":\"two\"}\n") + if err != nil { + t.Fatal(err) + } + + var messages []map[string]string + if err := json.Unmarshal([]byte(output), &messages); err != nil { + t.Fatal(err) + } + if len(messages) != 2 || messages[0]["account"] != "one" || messages[1]["account"] != "two" { + t.Fatalf("unexpected messages: %#v", messages) + } +} + +func TestMarshalJsonStreamRejectsTrailingStderr(t *testing.T) { + _, err := marshalJsonStream("{\"account\":\"one\"}\njava.lang.Throwable\n") + if err == nil { + t.Fatal("expected malformed trailing output to be rejected") + } +} + +func TestValidateBodyRangesUsesFinalUTF16Length(t *testing.T) { + parser := utils.NewTextstyleParser("👋 **hello**") + message, styles := parser.Parse() + mentions := []ds.MessageMention{{Start: 3, Length: 5, Author: "aci"}} + + if err := validateBodyRanges(message, mentions, styles); err != nil { + t.Fatal(err) + } +} + +func TestValidateBodyRangesRejectsUTF8ByteOffsets(t *testing.T) { + message := "👋 hello" + mentions := []ds.MessageMention{{Start: 5, Length: 5, Author: "aci"}} + + if err := validateBodyRanges(message, mentions, nil); err == nil { + t.Fatal("expected UTF-8 byte offset to exceed the UTF-16 message length") + } +} diff --git a/src/utils/textstyleparser.go b/src/utils/textstyleparser.go index 9b267c9d..ca58f4c4 100644 --- a/src/utils/textstyleparser.go +++ b/src/utils/textstyleparser.go @@ -27,7 +27,7 @@ const ( const EscapeCharacter rune = '\\' -func getUtf16StringLength(s string) int { +func UTF16StringLength(s string) int { runes := []rune(s) //turn string to slice length := 0 @@ -115,13 +115,13 @@ func (l *TextstyleParser) peek() rune { func (l *TextstyleParser) handleToken(tokenType int, signalCliStylingType string) { if l.tokens.Empty() { - l.tokens.Push(TokenState{BeginPos: getUtf16StringLength(l.fullString), Token: tokenType}) + l.tokens.Push(TokenState{BeginPos: UTF16StringLength(l.fullString), Token: tokenType}) } else { if l.tokens.Peek().Token == tokenType { tokenBeginState := l.tokens.Pop() - l.signalCliFormatStrings = append(l.signalCliFormatStrings, strconv.Itoa(tokenBeginState.BeginPos)+":"+strconv.Itoa(getUtf16StringLength(l.fullString)-tokenBeginState.BeginPos)+":"+signalCliStylingType) + l.signalCliFormatStrings = append(l.signalCliFormatStrings, strconv.Itoa(tokenBeginState.BeginPos)+":"+strconv.Itoa(UTF16StringLength(l.fullString)-tokenBeginState.BeginPos)+":"+signalCliStylingType) } else { - l.tokens.Push(TokenState{BeginPos: getUtf16StringLength(l.fullString), Token: tokenType}) + l.tokens.Push(TokenState{BeginPos: UTF16StringLength(l.fullString), Token: tokenType}) } } } From 090432e596fa4e3e898799ebf090701ef188d556 Mon Sep 17 00:00:00 2001 From: ziggy Date: Fri, 10 Jul 2026 18:16:19 +0300 Subject: [PATCH 2/2] Preserve INFO severity for signal-cli stderr --- src/client/cli.go | 21 ++++++++++++++++++--- src/client/cli_test.go | 10 ++++++++-- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/client/cli.go b/src/client/cli.go index 0c82bda2..4d516d5b 100644 --- a/src/client/cli.go +++ b/src/client/cli.go @@ -55,10 +55,25 @@ func classifySignalCliOutput(stdout string, stderr string) (string, string, stri output, infoMessages, warnMessages := stripInfoAndWarnMessages(stdout) stderr = strings.TrimSpace(stderr) if stderr != "" { - if warnMessages != "" { - warnMessages += "\n" + stderrOutput, stderrInfoMessages, stderrWarnMessages := stripInfoAndWarnMessages(stderr) + if stderrInfoMessages != "" { + if infoMessages != "" { + infoMessages += "\n" + } + infoMessages += stderrInfoMessages + } + if stderrWarnMessages != "" { + if warnMessages != "" { + warnMessages += "\n" + } + warnMessages += stderrWarnMessages + } + if stderrOutput != "" { + if warnMessages != "" { + warnMessages += "\n" + } + warnMessages += stderrOutput } - warnMessages += stderr } return output, infoMessages, warnMessages } diff --git a/src/client/cli_test.go b/src/client/cli_test.go index 05e6cd11..c2ffd3da 100644 --- a/src/client/cli_test.go +++ b/src/client/cli_test.go @@ -7,13 +7,19 @@ import ( func TestClassifySignalCliOutputKeepsStderrOutOfResponse(t *testing.T) { stdout := "{\"account\":\"+380000000001\"}\n" - stderr := "WARN IncomingMessageHandler - Invalid content! reason\njava.lang.Throwable\n\tat example" + stderr := "INFO Manager - Routine status\nWARN IncomingMessageHandler - Invalid content! reason\njava.lang.Throwable\n\tat example" - output, _, warnings := classifySignalCliOutput(stdout, stderr) + output, infos, warnings := classifySignalCliOutput(stdout, stderr) if output != strings.TrimSpace(stdout) { t.Fatalf("got output %q, wanted %q", output, strings.TrimSpace(stdout)) } + if !strings.Contains(infos, "INFO Manager - Routine status") { + t.Fatalf("INFO stderr was not preserved at INFO severity: %q", infos) + } + if strings.Contains(warnings, "INFO Manager - Routine status") { + t.Fatalf("INFO stderr was promoted to warning severity: %q", warnings) + } if !strings.Contains(warnings, "Invalid content! reason") || !strings.Contains(warnings, "java.lang.Throwable") { t.Fatalf("warnings did not preserve stderr: %q", warnings) }