From 6ed85b2c95e769dbbe0af8218b68eb6cf31cbc96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Thu, 7 Sep 2023 09:16:07 +0200 Subject: [PATCH 1/3] Sanitize JSON RPC response --- jsonrpc/jsonrpc.go | 4 +-- jsonrpc/jsonrpc_test.go | 56 +++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/jsonrpc/jsonrpc.go b/jsonrpc/jsonrpc.go index e5f3b18136..c3eea79dce 100644 --- a/jsonrpc/jsonrpc.go +++ b/jsonrpc/jsonrpc.go @@ -3,6 +3,7 @@ package jsonrpc import ( "encoding/json" "fmt" + "html/template" "io" "net" "net/http" @@ -313,11 +314,10 @@ func (j *JSONRPC) handleJSONRPCRequest(w http.ResponseWriter, req *http.Request) j.logger.Debug("handle", "request", string(data)) resp, err := j.dispatcher.Handle(data) - if err != nil { _, _ = w.Write([]byte(err.Error())) } else { - _, _ = w.Write(resp) + template.HTMLEscape(w, resp) } j.logger.Debug("handle", "response", string(resp)) diff --git a/jsonrpc/jsonrpc_test.go b/jsonrpc/jsonrpc_test.go index 4951d102d4..b906d7aa93 100644 --- a/jsonrpc/jsonrpc_test.go +++ b/jsonrpc/jsonrpc_test.go @@ -4,32 +4,21 @@ import ( "bytes" "encoding/json" "net" + "net/http/httptest" + "strings" "testing" "github.com/0xPolygon/polygon-edge/helper/tests" "github.com/0xPolygon/polygon-edge/versioning" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/hashicorp/go-hclog" ) func TestHTTPServer(t *testing.T) { - store := newMockStore() - port, portErr := tests.GetFreePort() - - if portErr != nil { - t.Fatalf("Unable to fetch free port, %v", portErr) - } - - config := &Config{ - Store: store, - Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: port}, - } - _, err := NewJSONRPC(hclog.NewNullLogger(), config) - - if err != nil { - t.Fatal(err) - } + _, err := newTestJSONRPC(t) + require.NoError(t, err) } func Test_handleGetRequest(t *testing.T) { @@ -66,3 +55,38 @@ func Test_handleGetRequest(t *testing.T) { response, ) } + +func TestHandleJSONRPCRequest_MaliciousInput(t *testing.T) { + t.Parallel() + + j, err := newTestJSONRPC(t) + require.NoError(t, err) + + maliciousInput := `` + + req := httptest.NewRequest("POST", "/eth_blockNumber", strings.NewReader(maliciousInput)) + req.Header.Set("Content-Type", "application/json") + + w := httptest.NewRecorder() + + j.handleJSONRPCRequest(w, req) + + responseBody := w.Body.String() + require.Contains(t, responseBody, "Invalid json request") +} + +func newTestJSONRPC(t *testing.T) (*JSONRPC, error) { + t.Helper() + + store := newMockStore() + + port, err := tests.GetFreePort() + require.NoError(t, err, "Unable to fetch free port, %v", err) + + config := &Config{ + Store: store, + Addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: port}, + } + + return NewJSONRPC(hclog.NewNullLogger(), config) +} From 02557c1fcb53c63cc73034c384c8b87d7fe013c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Thu, 7 Sep 2023 18:05:07 +0200 Subject: [PATCH 2/3] Logs --- jsonrpc/jsonrpc.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jsonrpc/jsonrpc.go b/jsonrpc/jsonrpc.go index c3eea79dce..fe57c65cf5 100644 --- a/jsonrpc/jsonrpc.go +++ b/jsonrpc/jsonrpc.go @@ -304,7 +304,9 @@ func (j *JSONRPC) handle(w http.ResponseWriter, req *http.Request) { func (j *JSONRPC) handleJSONRPCRequest(w http.ResponseWriter, req *http.Request) { data, err := io.ReadAll(req.Body) + fmt.Println("request body", string(data)) if err != nil { + fmt.Println("failed to read request data") _, _ = w.Write([]byte(err.Error())) return @@ -315,6 +317,7 @@ func (j *JSONRPC) handleJSONRPCRequest(w http.ResponseWriter, req *http.Request) resp, err := j.dispatcher.Handle(data) if err != nil { + fmt.Println("failed to handle request", err) _, _ = w.Write([]byte(err.Error())) } else { template.HTMLEscape(w, resp) From 740330d9fffdd6c5fbd40b8d76b7ae47724f77bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20Negovanovi=C4=87?= Date: Fri, 8 Sep 2023 09:05:30 +0200 Subject: [PATCH 3/3] Add another test case, revert HTML escape --- jsonrpc/jsonrpc.go | 6 +---- jsonrpc/jsonrpc_test.go | 51 ++++++++++++++++++++++++++++++----------- 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/jsonrpc/jsonrpc.go b/jsonrpc/jsonrpc.go index fe57c65cf5..bfae70b15b 100644 --- a/jsonrpc/jsonrpc.go +++ b/jsonrpc/jsonrpc.go @@ -3,7 +3,6 @@ package jsonrpc import ( "encoding/json" "fmt" - "html/template" "io" "net" "net/http" @@ -304,9 +303,7 @@ func (j *JSONRPC) handle(w http.ResponseWriter, req *http.Request) { func (j *JSONRPC) handleJSONRPCRequest(w http.ResponseWriter, req *http.Request) { data, err := io.ReadAll(req.Body) - fmt.Println("request body", string(data)) if err != nil { - fmt.Println("failed to read request data") _, _ = w.Write([]byte(err.Error())) return @@ -317,10 +314,9 @@ func (j *JSONRPC) handleJSONRPCRequest(w http.ResponseWriter, req *http.Request) resp, err := j.dispatcher.Handle(data) if err != nil { - fmt.Println("failed to handle request", err) _, _ = w.Write([]byte(err.Error())) } else { - template.HTMLEscape(w, resp) + _, _ = w.Write(resp) } j.logger.Debug("handle", "response", string(resp)) diff --git a/jsonrpc/jsonrpc_test.go b/jsonrpc/jsonrpc_test.go index b906d7aa93..32399ab84c 100644 --- a/jsonrpc/jsonrpc_test.go +++ b/jsonrpc/jsonrpc_test.go @@ -8,12 +8,11 @@ import ( "strings" "testing" - "github.com/0xPolygon/polygon-edge/helper/tests" - "github.com/0xPolygon/polygon-edge/versioning" - "github.com/stretchr/testify/assert" + "github.com/hashicorp/go-hclog" "github.com/stretchr/testify/require" - "github.com/hashicorp/go-hclog" + "github.com/0xPolygon/polygon-edge/helper/tests" + "github.com/0xPolygon/polygon-edge/versioning" ) func TestHTTPServer(t *testing.T) { @@ -40,12 +39,12 @@ func Test_handleGetRequest(t *testing.T) { response := &GetResponse{} - assert.NoError( + require.NoError( t, json.Unmarshal(mockWriter.Bytes(), response), ) - assert.Equal( + require.Equal( t, &GetResponse{ Name: chainName, @@ -56,23 +55,47 @@ func Test_handleGetRequest(t *testing.T) { ) } -func TestHandleJSONRPCRequest_MaliciousInput(t *testing.T) { +func TestJSONRPC_handleJSONRPCRequest(t *testing.T) { t.Parallel() j, err := newTestJSONRPC(t) require.NoError(t, err) - maliciousInput := `` + cases := []struct { + name string + request string + expectedResponse string + }{ + { + name: "malicious input (XSS attack)", + request: `{"jsonrpc":"2.0","id":0,"method":"eth_getBlockByNumber","params":["latest",false]} +`, + expectedResponse: `{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid json request"}}`, + }, + { + name: "valid input", + request: `{"jsonrpc":"2.0","id":0,"method":"eth_getBlockByNumber","params":["latest",false]}`, + expectedResponse: `{"jsonrpc":"2.0","id":0,"result":{`, + }, + } - req := httptest.NewRequest("POST", "/eth_blockNumber", strings.NewReader(maliciousInput)) - req.Header.Set("Content-Type", "application/json") + for _, c := range cases { + c := c - w := httptest.NewRecorder() + t.Run(c.name, func(t *testing.T) { + t.Parallel() - j.handleJSONRPCRequest(w, req) + req := httptest.NewRequest("POST", "/eth_getBlockByNumber", strings.NewReader(c.request)) + req.Header.Set("Content-Type", "application/json") - responseBody := w.Body.String() - require.Contains(t, responseBody, "Invalid json request") + w := httptest.NewRecorder() + + j.handleJSONRPCRequest(w, req) + + response := w.Body.String() + require.Contains(t, response, c.expectedResponse) + }) + } } func newTestJSONRPC(t *testing.T) (*JSONRPC, error) {