From b489b2128f53c6c8e94b206c9515fbb2988ae55f Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Sun, 9 Nov 2025 20:37:42 +0100 Subject: [PATCH] fix(vuln): fixed vulnerability in returned error, not escaping single quotes Signed-off-by: Frederic BIDON --- client_response.go | 7 +++- client_response_test.go | 79 +++++++++++++++++++++++++++++++---------- 2 files changed, 66 insertions(+), 20 deletions(-) diff --git a/client_response.go b/client_response.go index 2be38d2..f2cf942 100644 --- a/client_response.go +++ b/client_response.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "io" + "strings" ) // A ClientResponse represents a client response. @@ -50,13 +51,17 @@ func NewAPIError(opName string, payload any, code int) *APIError { } } +// sanitizer ensures that single quotes are escaped +var sanitizer = strings.NewReplacer(`\`, `\\`, `'`, `\'`) + func (o *APIError) Error() string { var resp []byte if err, ok := o.Response.(error); ok { - resp = []byte("'" + err.Error() + "'") + resp = []byte("'" + sanitizer.Replace(err.Error()) + "'") } else { resp, _ = json.Marshal(o.Response) } + return fmt.Sprintf("%s (status %d): %s", o.OperationName, o.Code, resp) } diff --git a/client_response_test.go b/client_response_test.go index 8a0169b..13c42a0 100644 --- a/client_response_test.go +++ b/client_response_test.go @@ -53,26 +53,67 @@ func TestResponseReaderFunc(t *testing.T) { assert.Equal(t, 490, actual.Code) } +type errResponse struct { + A int `json:"a"` + B string `json:"b"` +} + func TestResponseReaderFuncError(t *testing.T) { - reader := ClientResponseReaderFunc(func(r ClientResponse, _ Consumer) (any, error) { - _, _ = io.ReadAll(r.Body()) - return nil, NewAPIError("fake", errors.New("writer closed"), 490) + t.Parallel() + + t.Run("with API error as string", func(t *testing.T) { + reader := ClientResponseReaderFunc(func(r ClientResponse, _ Consumer) (any, error) { + _, _ = io.ReadAll(r.Body()) + + return nil, NewAPIError("fake", errors.New("writer closed"), 490) + }) + + _, err := reader.ReadResponse(response{}, nil) + require.Error(t, err) + require.ErrorContains(t, err, "'writer closed'") }) - _, err := reader.ReadResponse(response{}, nil) - require.Error(t, err) - require.ErrorContains(t, err, "writer closed") - - reader = func(r ClientResponse, _ Consumer) (any, error) { - _, _ = io.ReadAll(r.Body()) - err := &fs.PathError{ - Op: "write", - Path: "path/to/fake", - Err: fs.ErrClosed, - } - return nil, NewAPIError("fake", err, 200) - } - _, err = reader.ReadResponse(response{}, nil) - require.Error(t, err) - assert.Contains(t, err.Error(), "file already closed") + t.Run("with API error as complex error", func(t *testing.T) { + reader := ClientResponseReaderFunc(func(r ClientResponse, _ Consumer) (any, error) { + _, _ = io.ReadAll(r.Body()) + err := &fs.PathError{ + Op: "write", + Path: "path/to/fake", + Err: fs.ErrClosed, + } + + return nil, NewAPIError("fake", err, 200) + }) + + _, err := reader.ReadResponse(response{}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "file already closed") + }) + + t.Run("with API error requiring escaping", func(t *testing.T) { + reader := ClientResponseReaderFunc(func(r ClientResponse, _ Consumer) (any, error) { + _, _ = io.ReadAll(r.Body()) + return nil, NewAPIError("fake", errors.New(`writer is \"terminated\" and 'closed'`), 490) + }) + + _, err := reader.ReadResponse(response{}, nil) + require.Error(t, err) + require.ErrorContains(t, err, `'writer is \\"terminated\\" and \'closed\''`) + }) + + t.Run("with API error as JSON", func(t *testing.T) { + reader := ClientResponseReaderFunc(func(r ClientResponse, _ Consumer) (any, error) { + _, _ = io.ReadAll(r.Body()) + obj := &errResponse{ // does not implement error + A: 555, + B: "closed", + } + + return nil, NewAPIError("fake", obj, 200) + }) + + _, err := reader.ReadResponse(response{}, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), `{"a":555,"b":"closed"}`) + }) }