From a1fec4210438d01a6a7857ad4fc9681e854c71fc Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Mon, 29 Mar 2021 20:56:34 +0200 Subject: [PATCH 1/6] Handle malformed data in readAuthData --- server/handshake_resp.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/handshake_resp.go b/server/handshake_resp.go index 4edef88df..01697d9ed 100644 --- a/server/handshake_resp.go +++ b/server/handshake_resp.go @@ -129,6 +129,11 @@ func (c *Conn) readPluginName(data []byte, pos int) int { } func (c *Conn) readAuthData(data []byte, pos int) ([]byte, int, int, error) { + // prevent 'panic: runtime error: index out of range' error + if pos > len(data)-1 { + return nil, 0, 0, errors.New("bad handshake") + } + // length encoded data var auth []byte var authLen int From 1a5adb6f74bbb4da2299653f5548fd923d4f1611 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Mon, 29 Mar 2021 21:56:57 +0200 Subject: [PATCH 2/6] Return real mysql err --- server/handshake_resp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/handshake_resp.go b/server/handshake_resp.go index 01697d9ed..e3867ecb4 100644 --- a/server/handshake_resp.go +++ b/server/handshake_resp.go @@ -131,7 +131,7 @@ func (c *Conn) readPluginName(data []byte, pos int) int { func (c *Conn) readAuthData(data []byte, pos int) ([]byte, int, int, error) { // prevent 'panic: runtime error: index out of range' error if pos > len(data)-1 { - return nil, 0, 0, errors.New("bad handshake") + return nil, 0, 0, NewDefaultError(ER_HANDSHAKE_ERROR) } // length encoded data From 5d392c5877353cc73263c0d91820bdaceaaacc98 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Mon, 29 Mar 2021 21:57:18 +0200 Subject: [PATCH 3/6] Add test --- server/handshake_resp_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 server/handshake_resp_test.go diff --git a/server/handshake_resp_test.go b/server/handshake_resp_test.go new file mode 100644 index 000000000..041160aae --- /dev/null +++ b/server/handshake_resp_test.go @@ -0,0 +1,26 @@ +package server + +import ( + "testing" + + "github.com/siddontang/go-mysql/mysql" +) + +func TestReadAuthData(t *testing.T) { + c := &Conn{ + capability: mysql.CLIENT_SECURE_CONNECTION, + } + + data := []byte{141, 174, 255, 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 114, 111, 111, 116, 0, 20, 28, 152, 53, 72, 150, 120, 94, 151, 3, 104, 218, 30, 186, 82, 221, 123, 12, 50, 148, 88, 109, 121, 115, 113, 108, 95, 112, 101, 114, 102, 0, 109, 121, 115, 113, 108, 95, 110, 97, 116, 105, 118, 101, 95, 112, 97, 115, 115, 119, 111, 114, 100, 0} + + // test out of range index returns 'bad handshake' error + _, _, _, err := c.readAuthData(data, len(data)) + if err == nil || err.Error() != "ERROR 1043 (08S01): Bad handshake" { + t.Fatal("expected error, got nil") + } + + // test good index position reads auth data + if _, _, _, err := c.readAuthData(data, len(data)-1); err != nil { + t.Fatalf("expected nil error, got %v", err) + } +} From 2cffb196613ff81b4417096017fb52c6cf69dd27 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Mon, 29 Mar 2021 22:16:16 +0200 Subject: [PATCH 4/6] Update unit test --- server/handshake_resp_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/server/handshake_resp_test.go b/server/handshake_resp_test.go index 041160aae..a4956e3c2 100644 --- a/server/handshake_resp_test.go +++ b/server/handshake_resp_test.go @@ -3,15 +3,15 @@ package server import ( "testing" - "github.com/siddontang/go-mysql/mysql" + . "github.com/siddontang/go-mysql/mysql" ) func TestReadAuthData(t *testing.T) { c := &Conn{ - capability: mysql.CLIENT_SECURE_CONNECTION, + capability: CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA, } - data := []byte{141, 174, 255, 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 114, 111, 111, 116, 0, 20, 28, 152, 53, 72, 150, 120, 94, 151, 3, 104, 218, 30, 186, 82, 221, 123, 12, 50, 148, 88, 109, 121, 115, 113, 108, 95, 112, 101, 114, 102, 0, 109, 121, 115, 113, 108, 95, 110, 97, 116, 105, 118, 101, 95, 112, 97, 115, 115, 119, 111, 114, 100, 0} + data := []byte{141, 174, 255, 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 114, 111, 111, 116, 0, 20, 190, 183, 72, 209, 170, 60, 191, 100, 227, 81, 203, 221, 190, 14, 213, 116, 244, 140, 90, 121, 109, 121, 115, 113, 108, 95, 112, 101, 114, 102, 0, 109, 121, 115, 113, 108, 95, 110, 97, 116, 105, 118, 101, 95, 112, 97, 115, 115, 119, 111, 114, 100, 0} // test out of range index returns 'bad handshake' error _, _, _, err := c.readAuthData(data, len(data)) @@ -20,7 +20,11 @@ func TestReadAuthData(t *testing.T) { } // test good index position reads auth data - if _, _, _, err := c.readAuthData(data, len(data)-1); err != nil { + _, _, readBytes, err := c.readAuthData(data, len(data)-1) + if err != nil { t.Fatalf("expected nil error, got %v", err) } + if readBytes != len(data)-1 { + t.Fatalf("expected %d read bytes, got %d", len(data)-1, readBytes) + } } From c35a86a1914b7f677b17213cf0153d780824a39b Mon Sep 17 00:00:00 2001 From: atercattus Date: Thu, 27 May 2021 19:29:12 +0300 Subject: [PATCH 5/6] fix import in test --- server/handshake_resp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/handshake_resp_test.go b/server/handshake_resp_test.go index a4956e3c2..f1c7fdf4a 100644 --- a/server/handshake_resp_test.go +++ b/server/handshake_resp_test.go @@ -3,12 +3,12 @@ package server import ( "testing" - . "github.com/siddontang/go-mysql/mysql" + "github.com/go-mysql-org/go-mysql/mysql" ) func TestReadAuthData(t *testing.T) { c := &Conn{ - capability: CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA, + capability: mysql.CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA, } data := []byte{141, 174, 255, 1, 0, 0, 0, 1, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 114, 111, 111, 116, 0, 20, 190, 183, 72, 209, 170, 60, 191, 100, 227, 81, 203, 221, 190, 14, 213, 116, 244, 140, 90, 121, 109, 121, 115, 113, 108, 95, 112, 101, 114, 102, 0, 109, 121, 115, 113, 108, 95, 110, 97, 116, 105, 118, 101, 95, 112, 97, 115, 115, 119, 111, 114, 100, 0} From 9f60d5f96c725c5b8b2381a60c1050e4aa78af6c Mon Sep 17 00:00:00 2001 From: atercattus Date: Thu, 27 May 2021 19:36:25 +0300 Subject: [PATCH 6/6] catch any wrong position in Conn.readAuthData() --- server/handshake_resp.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/server/handshake_resp.go b/server/handshake_resp.go index 85bab5e74..e4f1e9be4 100644 --- a/server/handshake_resp.go +++ b/server/handshake_resp.go @@ -128,15 +128,15 @@ func (c *Conn) readPluginName(data []byte, pos int) int { return pos } -func (c *Conn) readAuthData(data []byte, pos int) ([]byte, int, int, error) { +func (c *Conn) readAuthData(data []byte, pos int) (auth []byte, authLen int, newPos int, err error) { // prevent 'panic: runtime error: index out of range' error - if pos > len(data)-1 { - return nil, 0, 0, NewDefaultError(ER_HANDSHAKE_ERROR) - } + defer func() { + if recover() != nil { + err = NewDefaultError(ER_HANDSHAKE_ERROR) + } + }() // length encoded data - var auth []byte - var authLen int if c.capability&CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA != 0 { authData, isNULL, readBytes, err := LengthEncodedString(data[pos:]) if err != nil { @@ -149,7 +149,7 @@ func (c *Conn) readAuthData(data []byte, pos int) ([]byte, int, int, error) { auth = authData authLen = readBytes } else if c.capability&CLIENT_SECURE_CONNECTION != 0 { - //auth length and auth + // auth length and auth authLen = int(data[pos]) pos++ auth = data[pos : pos+authLen]