Automatic Merge
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
21a86506f9
Коммит
51f2e2fdd5
@@ -982,6 +982,10 @@ func updatePost(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// MM-67055: Strip client-supplied metadata.embeds to prevent spoofing.
|
||||
// This matches createPost behavior.
|
||||
post.SanitizeInput()
|
||||
|
||||
auditRec := c.MakeAuditRecord("updatePost", model.AuditStatusFail)
|
||||
model.AddEventParameterAuditableToAuditRec(auditRec, "post", &post)
|
||||
defer c.LogAuditRecWithLevel(auditRec, app.LevelContent)
|
||||
|
||||
@@ -1450,6 +1450,53 @@ func TestUpdatePost(t *testing.T) {
|
||||
assert.NotEqual(t, rpost3.Attachments(), rrupost3.Attachments())
|
||||
})
|
||||
|
||||
t.Run("should strip spoofed metadata embeds", func(t *testing.T) {
|
||||
// MM-67055: Verify that client-supplied metadata.embeds are stripped
|
||||
post := &model.Post{
|
||||
ChannelId: channel.Id,
|
||||
Message: "test message " + model.NewId(),
|
||||
}
|
||||
createdPost, _, err := client.CreatePost(context.Background(), post)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Try to update with spoofed embed
|
||||
updatePost := &model.Post{
|
||||
Id: createdPost.Id,
|
||||
ChannelId: channel.Id,
|
||||
Message: "updated message " + model.NewId(),
|
||||
Metadata: &model.PostMetadata{
|
||||
Embeds: []*model.PostEmbed{
|
||||
{
|
||||
Type: model.PostEmbedPermalink,
|
||||
Data: &model.PreviewPost{
|
||||
PostID: "spoofed-post-id",
|
||||
Post: &model.Post{
|
||||
Id: "spoofed-post-id",
|
||||
UserId: th.BasicUser2.Id,
|
||||
Message: "This is a spoofed message!",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
updatedPost, _, err := client.UpdatePost(context.Background(), createdPost.Id, updatePost)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Verify spoofed embed was stripped
|
||||
if updatedPost.Metadata != nil {
|
||||
assert.Empty(t, updatedPost.Metadata.Embeds, "spoofed embeds should be stripped")
|
||||
}
|
||||
|
||||
// Double-check by fetching the post
|
||||
fetchedPost, _, err := client.GetPost(context.Background(), createdPost.Id, "")
|
||||
require.NoError(t, err)
|
||||
if fetchedPost.Metadata != nil {
|
||||
assert.Empty(t, fetchedPost.Metadata.Embeds, "spoofed embeds should not be persisted")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("change message, but post too old", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.ServiceSettings.PostEditTimeLimit = 1
|
||||
|
||||
@@ -795,6 +795,8 @@ func (a *App) UpdatePost(c request.CTX, receivedUpdatedPost *model.Post, updateP
|
||||
// Always use incoming metadata when provided, otherwise retain existing
|
||||
if receivedUpdatedPost.Metadata != nil {
|
||||
newPost.Metadata = receivedUpdatedPost.Metadata.Copy()
|
||||
// MM-67055: Strip embeds - always server-generated. Preserves Priority/Acks for Shared Channels sync.
|
||||
newPost.Metadata.Embeds = nil
|
||||
} else {
|
||||
// Restore the post metadata that was stripped by the plugin. Set it to
|
||||
// the last known good.
|
||||
|
||||
@@ -1791,6 +1791,60 @@ func TestUpdatePost(t *testing.T) {
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("should strip client-supplied embeds", func(t *testing.T) {
|
||||
// MM-67055: Verify that client-supplied metadata.embeds are stripped.
|
||||
// This prevents WebSocket message spoofing via permalink embeds.
|
||||
//
|
||||
// Note: Priority and Acknowledgements are stored in separate database tables,
|
||||
// not in post metadata. Shared Channels handles them separately via
|
||||
// syncRemotePriorityMetadata and syncRemoteAcknowledgementsMetadata after
|
||||
// calling UpdatePost. See sync_recv.go::upsertSyncPost
|
||||
mainHelper.Parallel(t)
|
||||
th := Setup(t).InitBasic()
|
||||
|
||||
th.AddUserToChannel(th.BasicUser, th.BasicChannel)
|
||||
th.Context.Session().UserId = th.BasicUser.Id
|
||||
|
||||
// Create a basic post
|
||||
post := &model.Post{
|
||||
ChannelId: th.BasicChannel.Id,
|
||||
Message: "original message",
|
||||
UserId: th.BasicUser.Id,
|
||||
}
|
||||
createdPost, _, err := th.App.CreatePost(th.Context, post, th.BasicChannel, model.CreatePostFlags{})
|
||||
require.Nil(t, err)
|
||||
|
||||
// Try to update with spoofed embeds (the attack vector)
|
||||
updatePost := &model.Post{
|
||||
Id: createdPost.Id,
|
||||
ChannelId: th.BasicChannel.Id,
|
||||
Message: "updated message",
|
||||
UserId: th.BasicUser.Id,
|
||||
Metadata: &model.PostMetadata{
|
||||
Embeds: []*model.PostEmbed{
|
||||
{
|
||||
Type: model.PostEmbedPermalink,
|
||||
Data: &model.PreviewPost{
|
||||
PostID: "spoofed-post-id",
|
||||
Post: &model.Post{
|
||||
Id: "spoofed-post-id",
|
||||
UserId: th.BasicUser2.Id,
|
||||
Message: "Spoofed message from another user!",
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
updatedPost, _, err := th.App.UpdatePost(th.Context, updatePost, nil)
|
||||
require.Nil(t, err)
|
||||
require.NotNil(t, updatedPost.Metadata)
|
||||
|
||||
// Verify embeds were stripped
|
||||
assert.Empty(t, updatedPost.Metadata.Embeds, "spoofed embeds should be stripped")
|
||||
})
|
||||
}
|
||||
|
||||
func TestSearchPostsForUser(t *testing.T) {
|
||||
|
||||
Ссылка в новой задаче
Block a user