From b0848b440b30f67a56eab313ec590cdf2bec8377 Mon Sep 17 00:00:00 2001 From: Georg Reinke Date: Fri, 14 Jan 2022 21:29:51 +0100 Subject: [PATCH 1/4] fix: check for potential encoding errors in message.IsValid --- message.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/message.go b/message.go index 16693eb..bdf43fd 100644 --- a/message.go +++ b/message.go @@ -208,7 +208,7 @@ func DecodeMessageWithFDs(rd io.Reader, fds []int) (msg *Message, err error) { // The possibly returned error can be an error of the underlying reader, an // InvalidMessageError or a FormatError. func DecodeMessage(rd io.Reader) (msg *Message, err error) { - return DecodeMessageWithFDs(rd, make([]int, 0)); + return DecodeMessageWithFDs(rd, make([]int, 0)) } type nullwriter struct{} @@ -227,8 +227,8 @@ func (msg *Message) CountFds() (int, error) { } func (msg *Message) EncodeToWithFDs(out io.Writer, order binary.ByteOrder) (fds []int, err error) { - if err := msg.IsValid(); err != nil { - return make([]int, 0), err + if err := msg.validateHeader(); err != nil { + return nil, err } var vs [7]interface{} switch order { @@ -237,7 +237,7 @@ func (msg *Message) EncodeToWithFDs(out io.Writer, order binary.ByteOrder) (fds case binary.BigEndian: vs[0] = byte('B') default: - return make([]int, 0), errors.New("dbus: invalid byte order") + return nil, errors.New("dbus: invalid byte order") } body := new(bytes.Buffer) fds = make([]int, 0) @@ -284,8 +284,13 @@ func (msg *Message) EncodeTo(out io.Writer, order binary.ByteOrder) (err error) } // IsValid checks whether msg is a valid message and returns an -// InvalidMessageError if it is not. +// InvalidMessageError or FormatError if it is not. func (msg *Message) IsValid() error { + var b bytes.Buffer + return msg.EncodeTo(&b, nativeEndian) +} + +func (msg *Message) validateHeader() error { if msg.Flags & ^(FlagNoAutoStart|FlagNoReplyExpected|FlagAllowInteractiveAuthorization) != 0 { return InvalidMessageError("invalid flags") } @@ -330,6 +335,7 @@ func (msg *Message) IsValid() error { return InvalidMessageError("missing signature") } } + return nil } From 6281a02cc3c1863314260a46fa8f9d93653df9e3 Mon Sep 17 00:00:00 2001 From: Georg Reinke Date: Fri, 14 Jan 2022 21:31:31 +0100 Subject: [PATCH 2/4] feat: add tests for message header validation --- message_test.go | 111 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 message_test.go diff --git a/message_test.go b/message_test.go new file mode 100644 index 0000000..d6c73da --- /dev/null +++ b/message_test.go @@ -0,0 +1,111 @@ +package dbus + +import "testing" + +func TestMessage_validateHeader(t *testing.T) { + var tcs = []struct { + msg Message + err error + }{ + { + msg: Message{ + Flags: 0xFF, + }, + err: InvalidMessageError("invalid flags"), + }, + { + msg: Message{ + Type: 0xFF, + }, + err: InvalidMessageError("invalid message type"), + }, + { + msg: Message{ + Type: TypeMethodCall, + Headers: map[HeaderField]Variant{ + 0xFF: MakeVariant("foo"), + }, + }, + err: InvalidMessageError("invalid header"), + }, + { + msg: Message{ + Type: TypeMethodCall, + Headers: map[HeaderField]Variant{ + FieldPath: MakeVariant(42), + }, + }, + err: InvalidMessageError("invalid type of header field"), + }, + { + msg: Message{ + Type: TypeMethodCall, + }, + err: InvalidMessageError("missing required header"), + }, + { + msg: Message{ + Type: TypeMethodCall, + Headers: map[HeaderField]Variant{ + FieldPath: MakeVariant(ObjectPath("break")), + FieldMember: MakeVariant("foo.bar"), + }, + }, + err: InvalidMessageError("invalid path name"), + }, + { + msg: Message{ + Type: TypeMethodCall, + Headers: map[HeaderField]Variant{ + FieldPath: MakeVariant(ObjectPath("/")), + FieldMember: MakeVariant("2"), + }, + }, + err: InvalidMessageError("invalid member name"), + }, + { + msg: Message{ + Type: TypeMethodCall, + Headers: map[HeaderField]Variant{ + FieldPath: MakeVariant(ObjectPath("/")), + FieldMember: MakeVariant("foo.bar"), + FieldInterface: MakeVariant("break"), + }, + }, + err: InvalidMessageError("invalid interface name"), + }, + { + msg: Message{ + Type: TypeError, + Headers: map[HeaderField]Variant{ + FieldReplySerial: MakeVariant(uint32(0)), + FieldErrorName: MakeVariant("break"), + }, + }, + err: InvalidMessageError("invalid error name"), + }, + { + + msg: Message{ + Type: TypeError, + Headers: map[HeaderField]Variant{ + FieldReplySerial: MakeVariant(uint32(0)), + FieldErrorName: MakeVariant("error.name"), + }, + Body: []interface{}{ + "break", + }, + }, + err: InvalidMessageError("missing signature"), + }, + } + + for _, tc := range tcs { + t.Run(tc.err.Error(), func(t *testing.T) { + err := tc.msg.validateHeader() + if err != tc.err { + t.Errorf("expected error %q, got %q", tc.err, err) + } + }) + } +} From e73aee4089c404dfe3552c160175e383264721ab Mon Sep 17 00:00:00 2001 From: Georg Reinke Date: Fri, 14 Jan 2022 21:34:03 +0100 Subject: [PATCH 3/4] fix: check validity of generated message in Emit --- export.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/export.go b/export.go index 5223347..bfcc8b9 100644 --- a/export.go +++ b/export.go @@ -216,21 +216,12 @@ func (conn *Conn) handleCall(msg *Message) { // Emit emits the given signal on the message bus. The name parameter must be // formatted as "interface.member", e.g., "org.freedesktop.DBus.NameLost". func (conn *Conn) Emit(path ObjectPath, name string, values ...interface{}) error { - if !path.IsValid() { - return errors.New("dbus: invalid object path") - } i := strings.LastIndex(name, ".") if i == -1 { return errors.New("dbus: invalid method name") } iface := name[:i] member := name[i+1:] - if !isValidMember(member) { - return errors.New("dbus: invalid method name") - } - if !isValidInterface(iface) { - return errors.New("dbus: invalid interface name") - } msg := new(Message) msg.Type = TypeSignal msg.Headers = make(map[HeaderField]Variant) @@ -241,6 +232,9 @@ func (conn *Conn) Emit(path ObjectPath, name string, values ...interface{}) erro if len(values) > 0 { msg.Headers[FieldSignature] = MakeVariant(SignatureOf(values...)) } + if err := msg.IsValid(); err != nil { + return err + } var closed bool conn.sendMessageAndIfClosed(msg, func() { From 65323e05ffdc93e9f824b805532600d4fcf22c3d Mon Sep 17 00:00:00 2001 From: Georg Reinke Date: Fri, 14 Jan 2022 21:41:11 +0100 Subject: [PATCH 4/4] fix: drop and log invalid messages --- export.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/export.go b/export.go index bfcc8b9..d3dd9f7 100644 --- a/export.go +++ b/export.go @@ -3,6 +3,7 @@ package dbus import ( "errors" "fmt" + "os" "reflect" "strings" ) @@ -209,7 +210,11 @@ func (conn *Conn) handleCall(msg *Message) { } reply.Headers[FieldSignature] = MakeVariant(SignatureOf(reply.Body...)) - conn.sendMessageAndIfClosed(reply, nil) + if err := reply.IsValid(); err != nil { + fmt.Fprintf(os.Stderr, "dbus: dropping invalid reply to %s.%s on obj %s: %s\n", ifaceName, name, path, err) + } else { + conn.sendMessageAndIfClosed(reply, nil) + } } }