Unverified Commit 5f57f486 authored by Florian Dewald's avatar Florian Dewald Committed by GitHub

fix(security): validate attachment filenames (#5218)

parent 1d7efb15
......@@ -64,6 +64,9 @@ func (s *APIV1Service) CreateAttachment(ctx context.Context, request *v1pb.Creat
if request.Attachment.Filename == "" {
return nil, status.Errorf(codes.InvalidArgument, "filename is required")
}
if !validateFilename(request.Attachment.Filename) {
return nil, status.Errorf(codes.InvalidArgument, "filename contains invalid characters or format")
}
if request.Attachment.Type == "" {
return nil, status.Errorf(codes.InvalidArgument, "type is required")
}
......@@ -325,6 +328,9 @@ func (s *APIV1Service) UpdateAttachment(ctx context.Context, request *v1pb.Updat
}
for _, field := range request.UpdateMask.Paths {
if field == "filename" {
if !validateFilename(request.Attachment.Filename) {
return nil, status.Errorf(codes.InvalidArgument, "filename contains invalid characters or format")
}
update.Filename = &request.Attachment.Filename
}
}
......@@ -701,3 +707,18 @@ func setResponseHeaders(ctx context.Context, headers map[string]string) error {
}
return grpc.SetHeader(ctx, metadata.Pairs(pairs...))
}
func validateFilename(filename string) bool {
// Reject path traversal attempts and make sure no additional directories are created
if !filepath.IsLocal(filename) || strings.ContainsAny(filename, "/\\") {
return false
}
// Reject filenames starting or ending with spaces or periods
if strings.HasPrefix(filename, " ") || strings.HasSuffix(filename, " ") ||
strings.HasPrefix(filename, ".") || strings.HasSuffix(filename, ".") {
return false
}
return true
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment