mirror of https://github.com/usememos/memos.git
fix: add access control checks for attachments, comments, and reactions
Security fixes for multiple authorization bypass vulnerabilities: - GetAttachment: Add visibility check via checkAttachmentAccess helper - UpdateAttachment: Add ownership check (creator or admin only) - Fileserver: Require creator/admin auth for unlinked attachments - ListMemoAttachments: Add memo visibility check - CreateMemoComment: Add memo visibility check for target memo - ListMemoReactions: Add memo visibility check - UpsertMemoReaction: Add memo visibility check All checks follow the existing pattern used in GetMemo for consistency.
This commit is contained in:
parent
86fab0cf4c
commit
c7b48b800f
|
|
@ -246,6 +246,12 @@ func (s *APIV1Service) GetAttachment(ctx context.Context, request *v1pb.GetAttac
|
|||
if attachment == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "attachment not found")
|
||||
}
|
||||
|
||||
// Check access permission based on linked memo visibility.
|
||||
if err := s.checkAttachmentAccess(ctx, attachment); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return convertAttachmentFromStore(attachment), nil
|
||||
}
|
||||
|
||||
|
|
@ -257,10 +263,24 @@ func (s *APIV1Service) UpdateAttachment(ctx context.Context, request *v1pb.Updat
|
|||
if request.UpdateMask == nil || len(request.UpdateMask.Paths) == 0 {
|
||||
return nil, status.Errorf(codes.InvalidArgument, "update mask is required")
|
||||
}
|
||||
user, err := s.fetchCurrentUser(ctx)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get current user: %v", err)
|
||||
}
|
||||
if user == nil {
|
||||
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
attachment, err := s.Store.GetAttachment(ctx, &store.FindAttachment{UID: &attachmentUID})
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get attachment: %v", err)
|
||||
}
|
||||
if attachment == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "attachment not found")
|
||||
}
|
||||
// Only the creator or admin can update the attachment.
|
||||
if attachment.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
|
||||
currentTs := time.Now().Unix()
|
||||
update := &store.UpdateAttachment{
|
||||
|
|
@ -549,6 +569,44 @@ func (s *APIV1Service) validateAttachmentFilter(ctx context.Context, filterStr s
|
|||
return nil
|
||||
}
|
||||
|
||||
// checkAttachmentAccess verifies the user has permission to access the attachment.
|
||||
// For unlinked attachments (no memo), only the creator can access.
|
||||
// For linked attachments, access follows the memo's visibility rules.
|
||||
func (s *APIV1Service) checkAttachmentAccess(ctx context.Context, attachment *store.Attachment) error {
|
||||
user, _ := s.fetchCurrentUser(ctx)
|
||||
|
||||
// For unlinked attachments, only the creator can access.
|
||||
if attachment.MemoID == nil {
|
||||
if user == nil {
|
||||
return status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
if attachment.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// For linked attachments, check memo visibility.
|
||||
memo, err := s.Store.GetMemo(ctx, &store.FindMemo{ID: attachment.MemoID})
|
||||
if err != nil {
|
||||
return status.Errorf(codes.Internal, "failed to get memo: %v", err)
|
||||
}
|
||||
if memo == nil {
|
||||
return status.Errorf(codes.NotFound, "memo not found")
|
||||
}
|
||||
|
||||
if memo.Visibility == store.Public {
|
||||
return nil
|
||||
}
|
||||
if user == nil {
|
||||
return status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
if memo.Visibility == store.Private && memo.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// shouldStripExif checks if the MIME type is an image format that may contain EXIF metadata.
|
||||
// Returns true for formats like JPEG, TIFF, WebP, HEIC, and HEIF which commonly contain
|
||||
// privacy-sensitive metadata such as GPS coordinates, camera settings, and device information.
|
||||
|
|
|
|||
|
|
@ -98,6 +98,24 @@ func (s *APIV1Service) ListMemoAttachments(ctx context.Context, request *v1pb.Li
|
|||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get memo: %v", err)
|
||||
}
|
||||
if memo == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "memo not found")
|
||||
}
|
||||
|
||||
// Check memo visibility.
|
||||
if memo.Visibility != store.Public {
|
||||
user, err := s.fetchCurrentUser(ctx)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get user: %v", err)
|
||||
}
|
||||
if user == nil {
|
||||
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
if memo.Visibility == store.Private && memo.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
}
|
||||
|
||||
attachments, err := s.Store.ListAttachments(ctx, &store.FindAttachment{
|
||||
MemoID: &memo.ID,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -551,6 +551,21 @@ func (s *APIV1Service) CreateMemoComment(ctx context.Context, request *v1pb.Crea
|
|||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get memo")
|
||||
}
|
||||
if relatedMemo == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "memo not found")
|
||||
}
|
||||
|
||||
// Check memo visibility before allowing comment.
|
||||
user, err := s.fetchCurrentUser(ctx)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get user")
|
||||
}
|
||||
if user == nil {
|
||||
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
if relatedMemo.Visibility == store.Private && relatedMemo.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
|
||||
// Create the memo comment first.
|
||||
memoComment, err := s.CreateMemo(ctx, &v1pb.CreateMemoRequest{
|
||||
|
|
|
|||
|
|
@ -15,6 +15,33 @@ import (
|
|||
)
|
||||
|
||||
func (s *APIV1Service) ListMemoReactions(ctx context.Context, request *v1pb.ListMemoReactionsRequest) (*v1pb.ListMemoReactionsResponse, error) {
|
||||
// Extract memo UID and check visibility.
|
||||
memoUID, err := ExtractMemoUIDFromName(request.Name)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.InvalidArgument, "invalid memo name: %v", err)
|
||||
}
|
||||
memo, err := s.Store.GetMemo(ctx, &store.FindMemo{UID: &memoUID})
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get memo: %v", err)
|
||||
}
|
||||
if memo == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "memo not found")
|
||||
}
|
||||
|
||||
// Check memo visibility.
|
||||
if memo.Visibility != store.Public {
|
||||
user, err := s.fetchCurrentUser(ctx)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get user")
|
||||
}
|
||||
if user == nil {
|
||||
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
if memo.Visibility == store.Private && memo.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
}
|
||||
|
||||
reactions, err := s.Store.ListReactions(ctx, &store.FindReaction{
|
||||
ContentID: &request.Name,
|
||||
})
|
||||
|
|
@ -40,6 +67,25 @@ func (s *APIV1Service) UpsertMemoReaction(ctx context.Context, request *v1pb.Ups
|
|||
if user == nil {
|
||||
return nil, status.Errorf(codes.Unauthenticated, "user not authenticated")
|
||||
}
|
||||
|
||||
// Extract memo UID and check visibility before allowing reaction.
|
||||
memoUID, err := ExtractMemoUIDFromName(request.Reaction.ContentId)
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.InvalidArgument, "invalid memo name: %v", err)
|
||||
}
|
||||
memo, err := s.Store.GetMemo(ctx, &store.FindMemo{UID: &memoUID})
|
||||
if err != nil {
|
||||
return nil, status.Errorf(codes.Internal, "failed to get memo: %v", err)
|
||||
}
|
||||
if memo == nil {
|
||||
return nil, status.Errorf(codes.NotFound, "memo not found")
|
||||
}
|
||||
|
||||
// Check memo visibility.
|
||||
if memo.Visibility == store.Private && memo.CreatorID != user.ID && !isSuperUser(user) {
|
||||
return nil, status.Errorf(codes.PermissionDenied, "permission denied")
|
||||
}
|
||||
|
||||
reaction, err := s.Store.UpsertReaction(ctx, &store.Reaction{
|
||||
CreatorID: user.ID,
|
||||
ContentID: request.Reaction.ContentId,
|
||||
|
|
|
|||
|
|
@ -469,8 +469,19 @@ func calculateThumbnailDimensions(width, height int) (int, int) {
|
|||
|
||||
// checkAttachmentPermission verifies the user has permission to access the attachment.
|
||||
func (s *FileServerService) checkAttachmentPermission(ctx context.Context, c echo.Context, attachment *store.Attachment) error {
|
||||
// For unlinked attachments, only the creator can access.
|
||||
if attachment.MemoID == nil {
|
||||
return nil // Unlinked attachments are accessible.
|
||||
user, err := s.getCurrentUser(ctx, c)
|
||||
if err != nil {
|
||||
return echo.NewHTTPError(http.StatusInternalServerError, "failed to get current user").SetInternal(err)
|
||||
}
|
||||
if user == nil {
|
||||
return echo.NewHTTPError(http.StatusUnauthorized, "unauthorized access")
|
||||
}
|
||||
if user.ID != attachment.CreatorID && user.Role != store.RoleAdmin {
|
||||
return echo.NewHTTPError(http.StatusForbidden, "forbidden access")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
memo, err := s.Store.GetMemo(ctx, &store.FindMemo{ID: attachment.MemoID})
|
||||
|
|
@ -493,7 +504,7 @@ func (s *FileServerService) checkAttachmentPermission(ctx context.Context, c ech
|
|||
return echo.NewHTTPError(http.StatusUnauthorized, "unauthorized access")
|
||||
}
|
||||
|
||||
if memo.Visibility == store.Private && user.ID != attachment.CreatorID {
|
||||
if memo.Visibility == store.Private && user.ID != memo.CreatorID && user.Role != store.RoleAdmin {
|
||||
return echo.NewHTTPError(http.StatusForbidden, "forbidden access")
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue