Problem with linked images' display in rst docs from trac

[mk.fg]

For example, it can be seen on ​docs/specifications/file-encoding.rst. Following code appears there:

<img alt="file-encoding1.svg" src="file-encoding1.svg" />

URL in "src" attribute now leads to HTML page of trac (with embedded image), but should lead to raw image itself for it to be rendered.

[davidsarah]

Hmm. I think the syntax will work for local files, and we don't want to break that. Perhaps the trac can be configured to serve those files raw, but I don't know whether it deliberately avoids doing that as an XSS defence.

[mk.fg]

I vaguely remember that the images were displayed on docs in the past, so I'd guess maybe some configuration bits from darcs plugin were not applied to git.

Ideally, I imagine it might have a whitelist of mime-types (like "image/*") or file patterns (like "*.{jpg,png,svg}") to serve raw, but lacking that I can imagine a following workaround (nginx config syntax):

location ~ ^/trac/tahoe-lafs/browser/git/docs/(\S+\.(svg|png|jpg|gif))$ {
  uwsgi_param REQUEST_URI /trac/tahoe-lafs/export/master/git/docs/$1;
  uwsgi_param PATH_INFO /trac/tahoe-lafs/export/master/git/docs/$1;

  uwsgi_pass unix:/run/uwsgi/trac.sock;
}

location / {
  ... block for the rest of the urls
}

(nginx seem to be advertised in the response headers)

I think it'd only introduce a little bit of unexpected behavior for users who might browse git tree and expect intermediate page for images, not the raw entities, but I think it shouldn't be that much of a problem either (browsers seem to handle images w/o html wrapping).

[zooko]

mk-fg: I'm willing to try some tweak to the nginx/uwsgi configuration, but what should it be, exactly? To make all .svg's and .jpg's be served raw by nginx? Or maybe configure trac to render them when it serves them?

[mk.fg]

Apologies for snippet being unhelpful, should've explained it with words.

The above tweak doesn't make nginx serve files as such, they are still served by trac, just as per the following link:

​https://tahoe-lafs.org/trac/tahoe-lafs/export/master/git/docs/specifications/file-encoding1.svg

What it does is for any uri like "/trac/tahoe-lafs/browser/git/docs/specifications/file-encoding1.svg" (matching regexp pattern in the location block) to pass uri like "/trac/tahoe-lafs/export/master/git/docs/specifications/file-encoding1.svg" to uwsgi and trac, so it will serve the raw file back to nginx.

I.e. it doesn't work around trac (which might be a bad idea, and probably not so easy to do for a bare git repo), so all the trac checks and tricks are in place, and it will only apply to "docs/" path in git.

I'm afraid I'm not aware of a way to make trac serve raw images by default and looking over git configuration section in a trac I maintain, I can't see any option to make it do that (as I think it did with darcs), but if such option exists, imho it might be a bit more consistent solution, though maybe a bit less convenient since it might be undesirable to display raw image attachments in wiki and tickets.

Does it make any sense?

[mk.fg]

Forgot to mention the downside, which probably should be considered.

Links like ​this one with images embedded have info about git revision these images came from and related links for revision/repo navigation, which will be inaccessible if such links will always be served raw.

Should still be possible to get to any revision by other means and pick the image there, but if this interface is commonly used (more so than docs with embedded images), proposed change might be undesirable.

[davidsarah]

Note that SVGs allow scripts, so serving them raw would in fact introduce an XSS vulnerability.

[davidsarah]

Replying to davidsarah:

Note that SVGs allow scripts, so serving them raw would in fact introduce an XSS vulnerability.

... which we may already be vulnerable to.

[mk.fg]

Replying to davidsarah:

Note that SVGs allow scripts, so serving them raw would in fact introduce an XSS vulnerability.

... which we may already be vulnerable to.

Whoa, you're right, didn't know about that at all.

But I still think benefits far outweight the risks in this case ;)

And it shouldn't be that dangerous coming from tahoe git. Trac shouldn't be that high-value target to worry about anything more sophisticated than wide-spectrum spam botnets, I think.

[mk.fg]

Replying to mk.fg:

Replying to davidsarah:

Note that SVGs allow scripts, so serving them raw would in fact introduce an XSS vulnerability.

... which we may already be vulnerable to.

Whoa, you're right, didn't know about that at all.

But I still think benefits far outweight the risks in this case ;)

Though maybe easy win-win solution would be to just convert these images to png and commit them like that, disallowing svg in trac from then on.

[zooko]

#2004 was a duplicate of this.