From: Jeff Halter <redacted>
Date: Wed, 4 Aug 2021 01:00:05 +0000 (-0700)
Subject: Fix string negation bug
X-Git-Url: https://git.r.bdr.sh/rbdr/mobius/commitdiff_plain/b25c4a19420c2fde1f290dd360c68b84e4eaa1ed

Fix string negation bug
---

diff --git a/hotline/account.go b/hotline/account.go
index b0e9ee5..bcacc73 100644
--- a/hotline/account.go
+++ b/hotline/account.go
@@ -45,7 +45,7 @@ func (a *Account) Payload() (out []byte) {
 
 		[]byte{0x00, 0x69}, // fieldUserLogin
 		loginLen,
-		[]byte(NegatedUserString([]byte(a.Login))),
+		negateString([]byte(a.Login)),
 
 		[]byte{0x00, 0x6e}, // fieldUserAccess
 		[]byte{0x00, 0x08},
diff --git a/hotline/client.go b/hotline/client.go
index fb3f4e7..b423f9f 100644
--- a/hotline/client.go
+++ b/hotline/client.go
@@ -612,8 +612,8 @@ func (c *Client) LogIn(login string, password string) error {
 			tranLogin, nil,
 			NewField(fieldUserName, []byte(c.pref.Username)),
 			NewField(fieldUserIconID, c.pref.IconBytes()),
-			NewField(fieldUserLogin, []byte(NegatedUserString([]byte(login)))),
-			NewField(fieldUserPassword, []byte(NegatedUserString([]byte(password)))),
+			NewField(fieldUserLogin, negateString([]byte(login))),
+			NewField(fieldUserPassword, negateString([]byte(password))),
 			NewField(fieldVersion, []byte{0, 2}),
 		),
 	)
diff --git a/hotline/server_blackbox_test.go b/hotline/server_blackbox_test.go
index b8990ad..41d331b 100644
--- a/hotline/server_blackbox_test.go
+++ b/hotline/server_blackbox_test.go
@@ -22,7 +22,7 @@ type testCase struct {
 }
 
 func (tt *testCase) Setup(srv *Server) error {
-	if err := srv.NewUser(tt.account.Login, tt.account.Name, NegatedUserString([]byte(tt.account.Password)), *tt.account.Access); err != nil {
+	if err := srv.NewUser(tt.account.Login, tt.account.Name, string(negateString([]byte(tt.account.Password))), *tt.account.Access); err != nil {
 		return err
 	}
 
diff --git a/hotline/transaction_handlers.go b/hotline/transaction_handlers.go
index b06297d..0ae6071 100644
--- a/hotline/transaction_handlers.go
+++ b/hotline/transaction_handlers.go
@@ -625,7 +625,6 @@ func HandleSetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error
 
 func HandleGetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error) {
 	userLogin := string(t.GetField(fieldUserLogin).Data)
-	decodedUserLogin := NegatedUserString(t.GetField(fieldUserLogin).Data)
 	account := cc.Server.Accounts[userLogin]
 	if account == nil {
 		errorT := cc.NewErrReply(t, "Account does not exist.")
@@ -635,7 +634,7 @@ func HandleGetUser(cc *ClientConn, t *Transaction) (res []Transaction, err error
 
 	res = append(res, cc.NewReply(t,
 		NewField(fieldUserName, []byte(account.Name)),
-		NewField(fieldUserLogin, []byte(decodedUserLogin)),
+		NewField(fieldUserLogin, negateString(t.GetField(fieldUserLogin).Data)),
 		NewField(fieldUserPassword, []byte(account.Password)),
 		NewField(fieldUserAccess, *account.Access),
 	))
diff --git a/hotline/user.go b/hotline/user.go
index f80fd72..5d09b2d 100644
--- a/hotline/user.go
+++ b/hotline/user.go
@@ -51,19 +51,20 @@ func ReadUser(b []byte) (*User, error) {
 
 // DecodeUserString decodes an obfuscated user string from a client
 // e.g. 98 8a 9a 8c 8b => "guest"
-func DecodeUserString(encodedString []byte) (decodedString string) {
-	for _, char := range encodedString {
-		decodedString += string(rune(255 - uint(char)))
+func DecodeUserString(obfuText []byte) (clearText string) {
+	for _, char := range obfuText {
+		clearText += string(rune(255 - uint(char)))
 	}
-	return decodedString
+	return clearText
 }
 
-// Take a []byte of uncoded ascii as input and encode it
-// TODO: change the method signature to take a string and return []byte
-func NegatedUserString(encodedString []byte) string {
-	var decodedString string
-	for _, char := range encodedString {
-		decodedString += string(255 - uint8(char))[1:]
+// negateString takes []byte s containing cleartext and rotates by 255 into obfuscated cleartext.
+// The Hotline protocol uses this format for sending passwords over network.
+// Not secure, but hey, it was the 90s!
+func negateString(clearText []byte) []byte {
+	obfuText := make([]byte, len(clearText))
+	for i := 0; i < len(clearText); i++ {
+		obfuText[i] = 255 - clearText[i]
 	}
-	return decodedString
+	return obfuText
 }
diff --git a/hotline/user_test.go b/hotline/user_test.go
index dae4274..705dec0 100644
--- a/hotline/user_test.go
+++ b/hotline/user_test.go
@@ -1,6 +1,7 @@
 package hotline
 
 import (
+	"bytes"
 	"github.com/stretchr/testify/assert"
 	"testing"
 )
@@ -90,20 +91,27 @@ func TestNegatedUserString(t *testing.T) {
 	tests := []struct {
 		name string
 		args args
-		want string
+		want []byte
 	}{
 		{
-			name: "encodes bytes to string",
+			name: "encodes bytes to expected string",
 			args: args{
 				encodedString: []byte("guest"),
 			},
-			want: string([]byte{0x98, 0x8a, 0x9a, 0x8c, 0x8b}),
+			want: []byte{0x98, 0x8a, 0x9a, 0x8c, 0x8b},
+		},
+		{
+			name: "encodes bytes with numerals to expected string",
+			args: args{
+				encodedString: []byte("foo1"),
+			},
+			want: []byte{0x99, 0x90, 0x90, 0xce },
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			if got := NegatedUserString(tt.args.encodedString); got != tt.want {
-				t.Errorf("NegatedUserString() = %v, want %v", got, tt.want)
+			if got := negateString(tt.args.encodedString); !bytes.Equal(got, tt.want) {
+				t.Errorf("NegatedUserString() = %x, want %x", got, tt.want)
 			}
 		})
 	}