GitLab

mentioned in merge request !3

merged

mentioned in commit slapos@499b0c8e

( the reactions is because we tried to reproduce the problem )

Hello @jerome. I've just noticed that this change broke rendering of SVG images for me:

https://navytux.spb.ru/~kirr/neo.html

Could you please add SVG back to types that are served with appropriate content type?

Thanks beforehand,
Kirill

Hello @kirr

Ah sorry for that. Unfortunately I am afraid that if we serve SVG with an appropriate content type, we have the same problem as with html, because SVG can contain javascript that would be executed in the context of lab.nexedi.com , with full access to authentication cookies.

To demonstrate the problem with html, we have been using an html page with javascript : https://lab.nexedi.com/jerome/test/raw/master/index.html . When clicking on this link, the embedded javascript is executed. This javascript was adding a reaction to this merge request as an example, but basically with this approach, an attacker with a repository on lab.nexedi.com can trick users in executing arbitrary code and because content is served from lab.nexedi.com , the requests are made with authentication cookies.

With latest gitlab on gitlab.com the svg seems to be served with content-type: image/svg+xml and content-disposition: attachment header, so they are downloaded and javascript is not executed https://gitlab.com/perrinjerome/test/-/raw/main/test.svg but they can not be used as an image:

On github the svg is served from another domain ( raw.githubusercontent.com ), so it does not have access to github.com authentication cookies and it is served as content-type: image/svg+xml without the content-disposition: attachment header, so it can be used as an image and because there's also a content-security-policy: default-src 'none'; style-src 'unsafe-inline'; sandbox header, the javascript is not executed when it is opened directly.

There was a discussion in https://github.com/w3c/svgwg/issues/266 where it was suggested to have another mime type for "SVG, but without javascript" but this idea was not accepted and instead the w3c guys said "use content security policy" header.

Newer versions gitlab have support for content security policy header, so in discussions with @romain and @alain.takoudjou we said that once we update gitlab we will try to enable it, so maybe we can revisit this later.

In the meantime, maybe this svg can be served from another domain ? for example erp5 has "Web Illustration" type in image module which removes the <scripts> from svg ( example https://www.erp5.com/web_page_module/14697?format= ), so web illustrations in published state would do.

PS: I don't know what I was thinking but I totally missed to @-mention you on this merge request, I should have let you know before, sorry about that.

Technically speaking, we could patch more, we could serve svg as svg and with a CSP, but the context of this MR was to restore the original gitlab behavior here ("patch less" :) ) so my personal preference would be that svg are served from somewhere else.

diff --git a/internal/git/xblob.go b/internal/git/xblob.go
index 2ad3f38ff71..40d27d492f5 100644
--- a/internal/git/xblob.go
+++ b/internal/git/xblob.go
@@ -208,6 +208,12 @@ func emitBlob(w http.ResponseWriter, repopath string, refpath string, r *http.Re
                w.Header().Set("Content-Type", "text/plain; charset=utf-8")
        } else if strings.HasPrefix(contentType, "image/") {
                w.Header().Set("Content-Type", contentType)
+       } else if mime.TypeByExtension(path.Ext(refpath)) == "image/svg+xml"  {
+               // Serve SVG with a content security policy to prevent execution of inline javascript
+               // note that the detection is a bit more complex 
+               // https://github.com/golang/go/issues/15888#issuecomment-222457018
+               w.Header().Set("Content-Type", "image/svg+xml; charset=utf-8")
+               w.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox")
        } else {
                w.Header().Set("Content-Type", "application/octet-stream")
        }

With latest gitlab on gitlab.com the svg seems to be served with content-type: image/svg+xml and content-disposition: attachment header, so they are downloaded and javascript is not executed https://gitlab.com/perrinjerome/test/-/raw/main/test.svg but they can not be used as an image ...

I was wrong, I made a mistake with the first version of that svg, the first line was a new line but now that I have fixed this, they can be included as an image.

They are served with:

< content-type: image/svg+xml
< content-disposition: attachment
< content-security-policy: ....

so I'm starting to think that the patch suggested above might be good, because it aligns the behavior with gitlab's original behavior.

If no objections, I will finalize this patch and change it to also set content-disposition: attachment header.

In general (ie, outside this specific gitlab context), I'm in favour of using the content-disposition: attachment header by default everywhere. And disable it only for the HTML page of gitlab UI.

I don't know how are doing this could be in the context of gitlab

@jerome, thanks for explaining all this. I understand the rationale for the change, and I understand that it was kind of normal not to cc me since I'm no longer actively working on gitlab topic nowdays.

If it is indeed possible to restore SVG serving as image with Content-Security-Policy or content-disposition: attachment that would be very good. Besides that https://navytux.spb.ru/~kirr/neo.html I have also posted on the other places many links to other images hosted on our lab, and all them might be not rendered correctly now.

If we can fix SVG rendering back without sacrificing security, let's do that please.

mentioned in merge request !5 (merged)

WIP: no content type