Validate message body ranges and harden receive JSON handling

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,34 @@ 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 != "" {
+		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
+		}
+	}
+	return output, infoMessages, warnMessages
+}
+
 func (s *CliClient) Execute(wait bool, args []string, stdin string) (string, error) {
 	containerId, err := getContainerId()
 
@@ -132,17 +161,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)

src/client/cli_test.go

@@ -0,0 +1,26 @@
+package client
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestClassifySignalCliOutputKeepsStderrOutOfResponse(t *testing.T) {
+	stdout := "{\"account\":\"+380000000001\"}\n"
+	stderr := "INFO Manager - Routine status\nWARN IncomingMessageHandler - Invalid content! reason\njava.lang.Throwable\n\tat example"
+
+	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)
+	}
+}

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) {

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")
+	}
+}

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})
 		}
 	}
 }