fix(broker): allow control characters in environment values (#285)

3 files · +11 -3

M internal/adapters/daemonmux/codec_test.go +2 -2


@@ -527,7 +527,7 @@ func TestMuxValidationNegatives(t *testing.T) {
527	527		_, err := EncodeServer(Refused{Ref: testRef(5), Error: boundary}, testEnvelopeCeiling, testChunkCeiling)
528	528		require.NoError(t, err)
529	529		})
530		-	t.Run("bidi and control text refused", func(t *testing.T) {
	530	+	t.Run("display control text and env NUL refused", func(t *testing.T) {
531	531		evil := testErrorDetail()
532	532		evil.Text = "timeout\u202erorrE"
533	533		_, err := EncodeServer(Refused{Ref: testRef(5), Error: evil}, testEnvelopeCeiling, testChunkCeiling)

@@ -537,7 +537,7 @@ func TestMuxValidationNegatives(t *testing.T) {
537	537		require.ErrorIs(t, err, ErrInvalidMessage)
538	538
539	539		badEnv := testOpen()
540		-	badEnv.Env = []string{"K=\u202eevil"}
	540	+	badEnv.Env = []string{"K=bad\x00value"}
541	541		_, err = EncodeClient(badEnv, testEnvelopeCeiling, testChunkCeiling)
542	542		require.ErrorIs(t, err, ErrInvalidMessage)
543	543		})


@@ -527,7 +527,7 @@ func TestMuxValidationNegatives(t *testing.T) {
527		_, err := EncodeServer(Refused{Ref: testRef(5), Error: boundary}, testEnvelopeCeiling, testChunkCeiling)	527		_, err := EncodeServer(Refused{Ref: testRef(5), Error: boundary}, testEnvelopeCeiling, testChunkCeiling)
528		require.NoError(t, err)	528		require.NoError(t, err)
529		})	529		})
530	-	t.Run("bidi and control text refused", func(t *testing.T) {	530	+	t.Run("display control text and env NUL refused", func(t *testing.T) {
531		evil := testErrorDetail()	531		evil := testErrorDetail()
532		evil.Text = "timeout\u202erorrE"	532		evil.Text = "timeout\u202erorrE"
533		_, err := EncodeServer(Refused{Ref: testRef(5), Error: evil}, testEnvelopeCeiling, testChunkCeiling)	533		_, err := EncodeServer(Refused{Ref: testRef(5), Error: evil}, testEnvelopeCeiling, testChunkCeiling)

@@ -537,7 +537,7 @@ func TestMuxValidationNegatives(t *testing.T) {
537		require.ErrorIs(t, err, ErrInvalidMessage)	537		require.ErrorIs(t, err, ErrInvalidMessage)
538			538
539		badEnv := testOpen()	539		badEnv := testOpen()
540	-	badEnv.Env = []string{"K=\u202eevil"}	540	+	badEnv.Env = []string{"K=bad\x00value"}
541		_, err = EncodeClient(badEnv, testEnvelopeCeiling, testChunkCeiling)	541		_, err = EncodeClient(badEnv, testEnvelopeCeiling, testChunkCeiling)
542		require.ErrorIs(t, err, ErrInvalidMessage)	542		require.ErrorIs(t, err, ErrInvalidMessage)
543		})	543		})

M internal/adapters/protoconv/broker_convert.go +7 -1


@@ -130,7 +130,13 @@ func BrokerDisplayText(value string, maxBytes int) error {
130	130		}
131	131
132	132		func BrokerEnvEntry(entry string) error {
133		-	return BrokerDisplayText(entry, ports.BrokerMaxEnvEntryBytes)
	133	+	if len(entry) > ports.BrokerMaxEnvEntryBytes {
	134	+	return ErrTooLarge
	135	+	}
	136	+	if !utf8.ValidString(entry) \|\| strings.ContainsRune(entry, 0) {
	137	+	return ErrInvalid
	138	+	}
	139	+	return nil
134	140		}
135	141
136	142		func BrokerEnvEntries(env []string) error {

 }
 func BrokerEnvEntry(entry string) error {
-	return BrokerDisplayText(entry, ports.BrokerMaxEnvEntryBytes)
+	if len(entry) > ports.BrokerMaxEnvEntryBytes {
+		return ErrTooLarge
+	}
+	if !utf8.ValidString(entry) || strings.ContainsRune(entry, 0) {
+		return ErrInvalid
+	}
+	return nil
 }
 func BrokerEnvEntries(env []string) error {

M internal/adapters/protoconv/broker_convert_test.go +2 -0


@@ -98,6 +98,8 @@ func TestBrokerConversionFailures(t *testing.T) {
98	98		{"invalid utf8", func() error { return BrokerDisplayText("\xff", 20) }, ErrInvalid},
99	99		{"oversize env", func() error { return BrokerEnvEntries([]string{strings.Repeat("a", ports.BrokerMaxEnvEntryBytes+1)}) }, ErrTooLarge},
100	100		{"missing equals", func() error { return BrokerEnvEntries([]string{"TERM"}) }, ErrInvalid},
	101	+	{"nul env", func() error { return BrokerEnvEntries([]string{"A=x\x00y"}) }, ErrInvalid},
	102	+	{"ansi prompt env", func() error { return BrokerEnvEntries([]string{"PS1=\x1b[32m$ \x1b[0m"}) }, nil},
101	103		{"valid env", func() error { return BrokerEnvEntries([]string{"TERM=x"}) }, nil},
102	104		} {
103	105		t.Run(tc.name, func(t *testing.T) {


@@ -98,6 +98,8 @@ func TestBrokerConversionFailures(t *testing.T) {
98	{"invalid utf8", func() error { return BrokerDisplayText("\xff", 20) }, ErrInvalid},	98		{"invalid utf8", func() error { return BrokerDisplayText("\xff", 20) }, ErrInvalid},
99	{"oversize env", func() error { return BrokerEnvEntries([]string{strings.Repeat("a", ports.BrokerMaxEnvEntryBytes+1)}) }, ErrTooLarge},	99		{"oversize env", func() error { return BrokerEnvEntries([]string{strings.Repeat("a", ports.BrokerMaxEnvEntryBytes+1)}) }, ErrTooLarge},
100	{"missing equals", func() error { return BrokerEnvEntries([]string{"TERM"}) }, ErrInvalid},	100		{"missing equals", func() error { return BrokerEnvEntries([]string{"TERM"}) }, ErrInvalid},
		101	+	{"nul env", func() error { return BrokerEnvEntries([]string{"A=x\x00y"}) }, ErrInvalid},
		102	+	{"ansi prompt env", func() error { return BrokerEnvEntries([]string{"PS1=\x1b[32m$ \x1b[0m"}) }, nil},
101	{"valid env", func() error { return BrokerEnvEntries([]string{"TERM=x"}) }, nil},	103		{"valid env", func() error { return BrokerEnvEntries([]string{"TERM=x"}) }, nil},
102	} {	104		} {
103	t.Run(tc.name, func(t *testing.T) {	105		t.Run(tc.name, func(t *testing.T) {