Used limited reader in user invite (#25943)

* Used limited reader in user invite * Added tests
mattermost · Jan 23, 2024 · 62064e3 · 62064e3
1 parent 09b7012
commit 62064e3
Show file tree

Hide file tree

Showing 5 changed files with 175 additions and 18 deletions.
diff --git a/server/channels/api4/team.go b/server/channels/api4/team.go
@@ -1375,16 +1375,12 @@ func inviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	bf, err := io.ReadAll(r.Body)
+	memberInvite := &model.MemberInvite{}
+	err := model.StructFromJSONLimited(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes, memberInvite)
 	if err != nil {
 		c.Err = model.NewAppError("Api4.inviteUsersToTeams", "api.team.invite_members_to_team_and_channels.invalid_body.app_error", nil, "", http.StatusBadRequest).Wrap(err)
 		return
 	}
-	memberInvite := &model.MemberInvite{}
-	if err := json.Unmarshal(bf, memberInvite); err != nil {
-		c.Err = model.NewAppError("Api4.inviteUsersToTeams", "api.team.invite_members_to_team_and_channels.invalid_body_parsing.app_error", nil, "", http.StatusBadRequest).Wrap(err)
-		return
-	}
 
 	emailList := memberInvite.Emails
 

diff --git a/server/channels/api4/team_local.go b/server/channels/api4/team_local.go
@@ -6,7 +6,6 @@ package api4
 import (
 	"encoding/json"
 	"fmt"
-	"io"
 	"net/http"
 	"strings"
 
@@ -79,15 +78,10 @@ func localInviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	bf, err := io.ReadAll(r.Body)
-	if err != nil {
-		c.Err = model.NewAppError("Api4.inviteUsersToTeams", "api.team.invite_members_to_team_and_channels.invalid_body.app_error", nil, "", http.StatusBadRequest).Wrap(err)
-		return
-	}
 	memberInvite := &model.MemberInvite{}
-	err = json.Unmarshal(bf, memberInvite)
+	err := model.StructFromJSONLimited(r.Body, *c.App.Config().ServiceSettings.MaximumPayloadSizeBytes, memberInvite)
 	if err != nil {
-		c.Err = model.NewAppError("Api4.inviteUsersToTeams", "api.team.invite_members_to_team_and_channels.invalid_body_parsing.app_error", nil, "", http.StatusBadRequest).Wrap(err)
+		c.Err = model.NewAppError("Api4.localInviteUsersToTeam", "api.team.invite_members_to_team_and_channels.invalid_body.app_error", nil, "", http.StatusBadRequest).Wrap(err)
 		return
 	}
 

diff --git a/server/i18n/en.json b/server/i18n/en.json
@@ -3262,10 +3262,6 @@
     "id": "api.team.invite_members_to_team_and_channels.invalid_body.app_error",
     "translation": "Invalid request body."
   },
-  {
-    "id": "api.team.invite_members_to_team_and_channels.invalid_body_parsing.app_error",
-    "translation": "Error while parsing the body data."
-  },
   {
     "id": "api.team.is_team_creation_allowed.disabled.app_error",
     "translation": "Team creation has been disabled. Please ask your System Administrator for details."

diff --git a/server/public/model/utils.go b/server/public/model/utils.go
@@ -553,6 +553,16 @@ func StringInterfaceFromJSON(data io.Reader) map[string]any {
 	return objmap
 }
 
+func StructFromJSONLimited[V any](data io.Reader, maxBytes int64, obj *V) error {
+	lr := io.LimitReader(data, maxBytes)
+	err := json.NewDecoder(lr).Decode(&obj)
+	if err != nil || obj == nil {
+		return err
+	}
+
+	return nil
+}
+
 // ToJSON serializes an arbitrary data type to JSON, discarding the error.
 func ToJSON(v any) []byte {
 	b, _ := json.Marshal(v)

diff --git a/server/public/model/utils_test.go b/server/public/model/utils_test.go
@@ -1207,3 +1207,164 @@ func TestRemoveDuplicateStrings(t *testing.T) {
 		require.Equalf(t, actual, tc.Result, "case: %v\tshould returned: %#v", tc, tc.Result)
 	}
 }
+
+func TestStructFromJSONLimited(t *testing.T) {
+	t.Run("successfully parses basic struct", func(t *testing.T) {
+		type TestStruct struct {
+			StringField string
+			IntField    int
+			FloatField  float32
+			BoolField   bool
+		}
+
+		testStruct := TestStruct{
+			StringField: "string",
+			IntField:    2,
+			FloatField:  3.1415,
+			BoolField:   true,
+		}
+		testStructBytes, err := json.Marshal(testStruct)
+		require.NoError(t, err)
+
+		b := &TestStruct{}
+		err = StructFromJSONLimited(bytes.NewReader(testStructBytes), 1000, b)
+		require.NoError(t, err)
+
+		require.Equal(t, b.StringField, "string")
+		require.Equal(t, b.IntField, 2)
+		require.Equal(t, b.FloatField, float32(3.1415))
+		require.Equal(t, b.BoolField, true)
+	})
+
+	t.Run("error too big", func(t *testing.T) {
+		type TestStruct struct {
+			StringField string
+			IntField    int
+			FloatField  float32
+			BoolField   bool
+		}
+
+		testStruct := TestStruct{
+			StringField: "string",
+			IntField:    2,
+			FloatField:  3.1415,
+			BoolField:   true,
+		}
+		testStructBytes, err := json.Marshal(testStruct)
+		require.NoError(t, err)
+
+		b := &TestStruct{}
+		err = StructFromJSONLimited(bytes.NewReader(testStructBytes), 10, b)
+		require.Error(t, err)
+		require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+	})
+
+	t.Run("successfully parses nested struct", func(t *testing.T) {
+		type TestStruct struct {
+			StringField string
+			IntField    int
+			FloatField  float32
+			BoolField   bool
+		}
+
+		type NestedStruct struct {
+			FieldA TestStruct
+			FieldB TestStruct
+			FieldC []int
+		}
+
+		testStructA := TestStruct{
+			StringField: "string A",
+			IntField:    2,
+			FloatField:  3.1415,
+			BoolField:   true,
+		}
+
+		testStructB := TestStruct{
+			StringField: "string B",
+			IntField:    3,
+			FloatField:  100,
+			BoolField:   false,
+		}
+
+		nestedStruct := NestedStruct{
+			FieldA: testStructA,
+			FieldB: testStructB,
+			FieldC: []int{5, 9, 1, 5, 7},
+		}
+
+		nestedStructBytes, err := json.Marshal(nestedStruct)
+		require.NoError(t, err)
+
+		b := &NestedStruct{}
+		err = StructFromJSONLimited(bytes.NewReader(nestedStructBytes), 1000, b)
+		require.NoError(t, err)
+
+		require.Equal(t, b.FieldA.StringField, "string A")
+		require.Equal(t, b.FieldA.IntField, 2)
+		require.Equal(t, b.FieldA.FloatField, float32(3.1415))
+		require.Equal(t, b.FieldA.BoolField, true)
+
+		require.Equal(t, b.FieldB.StringField, "string B")
+		require.Equal(t, b.FieldB.IntField, 3)
+		require.Equal(t, b.FieldB.FloatField, float32(100))
+		require.Equal(t, b.FieldB.BoolField, false)
+
+		require.Equal(t, b.FieldC, []int{5, 9, 1, 5, 7})
+	})
+
+	t.Run("errors on too big nested struct", func(t *testing.T) {
+		type TestStruct struct {
+			StringField string
+			IntField    int
+			FloatField  float32
+			BoolField   bool
+		}
+
+		type NestedStruct struct {
+			FieldA TestStruct
+			FieldB TestStruct
+			FieldC []int
+		}
+
+		testStructA := TestStruct{
+			StringField: "string A",
+			IntField:    2,
+			FloatField:  3.1415,
+			BoolField:   true,
+		}
+
+		testStructB := TestStruct{
+			StringField: "string B",
+			IntField:    3,
+			FloatField:  100,
+			BoolField:   false,
+		}
+
+		nestedStruct := NestedStruct{
+			FieldA: testStructA,
+			FieldB: testStructB,
+			FieldC: []int{5, 9, 1, 5, 7},
+		}
+
+		nestedStructBytes, err := json.Marshal(nestedStruct)
+		require.NoError(t, err)
+
+		b := &NestedStruct{}
+		err = StructFromJSONLimited(bytes.NewReader(nestedStructBytes), 50, b)
+		require.Error(t, err)
+		require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+	})
+
+	t.Run("handles empty structs", func(t *testing.T) {
+		type TestStruct struct{}
+
+		testStruct := TestStruct{}
+		testStructBytes, err := json.Marshal(testStruct)
+		require.NoError(t, err)
+
+		b := &TestStruct{}
+		err = StructFromJSONLimited(bytes.NewReader(testStructBytes), 1000, b)
+		require.NoError(t, err)
+	})
+}