View Issue Details

IDProjectCategoryView StatusLast Update
0012313mantisbtattachmentspublic2017-09-03 18:41
ReporterAvg00rAssigned Todregad 
PrioritynormalSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.2 
Target Version2.6.0Fixed in Version2.6.0 
Summary0012313: Can't open image attachments in browser windows
Description

Actual result: Clicking on attachment link (on page "view.php") rise "save as"-dialog in a browser (IE, Opera, Google Chrome, Firefox).

Expected result: Open image in a browser (in new\same window).

Additional Information

Happened after update from 1.2.1 to 1.2.2.

TagsNo tags attached.

Relationships

related to 0011952 closeddhx Arbitrary inline attachment rendering could lead to cross-domain scripting or other browser attacks 
has duplicate 0013700 closedrombert "Rather than downloading, the attachment is viewed in the browser" doesn't work 
has duplicate 0014722 closedatrol Stop downloading image attachements 
has duplicate 0015683 closeddregad image attachment now showing in browser 
has duplicate 0022543 closedvboctor Open images in the browser rather than download them 

Activities

atrol

atrol

2010-09-01 10:34

developer   ~0026540

Reminder sent to: dhx

is this behaviour now WAD since fix of 0011952 ?

dhx

dhx

2010-09-01 20:03

reporter   ~0026545

You're right atrol, this change was made intentionally for security reasons. Therefore nothing needs to be fixed. If users click on an attachment name they should be presented with a file download dialog. The old method attempted to guess the MIME type and let the browser try to render it (opening up the possibility for XSS attacks - or worse).

Avg00r: I suggest enabling inline image previews as this is a more secure way of showing image attachments in the browser. If we ever want to improve the ability for users to view image attachments inline, this is the feature we'd look to improve.

Avg00r

Avg00r

2010-09-02 07:38

reporter   ~0026551

Last edited: 2010-09-02 14:54

View 2 revisions

dhx, I guess it is right to let users to choose between security and usability.
I made some fixes to rollback changes from 0011952 :) but it will be great if there is a property to ON\OFF inline attachment rendering.
Is it possible?

wmadmin

wmadmin

2010-09-09 07:12

reporter   ~0026642

Avg00r, can you provide me fixed file version ?

vtheiding

vtheiding

2010-09-12 06:17

reporter   ~0026677

I want to second Avg00r's request. Since our Mantis is only used internally, we prefer usability.
After looking at the code changes, I reinserted the code that sets $t_show_inline (now $f_show_inline) from 1.2.1 into file_download.php. Seems to work, at least in a quick test.
If my understanding of the code is correct, it should be sufficient to leave the configuration variable inline_file_exts empty to avoid the security risks. Therefore, I suggest putting the code back in, and instead to change the default in config_defaults_inc.php to empty and to add a note about the possible security risks that setting this variable implies.
BTW: is inline_file_exts used in 1.2.2 anymore?

hanoii

hanoii

2010-10-01 13:22

reporter   ~0026931

If you right-click on the image preview and do an open image on a new tab (chrome at least) you get to see the image, because it contains all the security additions on the url.

I wonder if clicking on the preview image or on the clicket should link to that instead of to the download link? Is that the exposing the security issue again?

In anyway, I second the approach of having that configurable for controlled (intranet) bugtracking

Avg00r

Avg00r

2010-10-04 09:41

reporter   ~0026943

Last edited: 2017-05-23 08:09

View 3 revisions

There are changes for 1.2.3 version.
First, add this variable to your config_inc.php:

# Fix 12313 usability issue and rollback changes for security issue 11952 fix.
# http://www.mantisbt.org/bugs/view.php?id=12313 [^]
# http://www.mantisbt.org/bugs/view.php?id=11952 [^]
# ON - you can open images in new browser window by click on it. 
# OFF - you can't. But you are secured.
$g_allow_inline_attachment_rendering = ON;

The following changes I've put 1.2.3's version code in the else-block, and a code of 1.2.1 inside if-block.

file_download.php. Two changes. First one:

if( config_get( 'allow_inline_attachment_rendering' ) == ON ) { 
    # There are variables from 1.2.1 before fix 11952 and about 12313 issues:
    $f_file_id  = gpc_get_int( 'file_id' );
    $f_type  = gpc_get_string( 'type' );
} else {
    # 1.2.2 security fix 11952 and about 12313 issues:  
    $f_show_inline = gpc_get_bool( 'show_inline', false );
    # To prevent cross-domain inline hotlinking to attachments we require a CSRF
    # token from the user to show any attachment inline within the browser.
    # Without this security in place a malicious user could upload a HTML file
    # attachment and direct a user to file_download.php?file_id=X&type=bug&show_inline=1
    # and the malicious HTML content would be rendered in the user's browser,
    # violating cross-domain security.
    if ( $f_show_inline ) {
        # Disable errors for form_security_validate as we need to send HTTP
        # headers prior to raising an error (the error handler within
        # error_api.php doesn't check that headers have been sent, it just
        # makes the assumption that they've been sent already).
        if ( !@form_security_validate( 'file_show_inline' ) ) {
            http_all_headers();
            trigger_error( ERROR_FORM_TOKEN_INVALID, ERROR );
        }
    }
    $f_file_id = gpc_get_int( 'file_id' );
    $f_type = gpc_get_string( 'type' );
}

And second:

if( config_get( 'allow_inline_attachment_rendering' ) == ON ) {
    # This code from 1.2.1 before fix 11952 and about 12313 issues:
    $t_show_inline = false;
    $t_inline_files = explode( ',', config_get( 'inline_file_exts' ) );
    if ( $t_inline_files !== false && !is_blank( $t_inline_files[0] ) ) {
        if ( in_array( utf8_strtolower( file_get_extension( $t_filename ) ), $t_inline_files ) ) {
            $t_show_inline = true;
        }
    }   
    http_content_disposition_header( $t_filename, $t_show_inline );
} else {
    # 1.2.2 security fix 11952 and about 12313 issues:  
    # For Internet Explorer 8 as per http://blogs.msdn.com/ie/archive/2008/07/02/ie8-security-part-v-comprehensive-protection.aspx [^]
    # Don't let IE second guess our content-type!
    header( 'X-Content-Type-Options: nosniff' );
    http_content_disposition_header( $t_filename, $f_show_inline );
}

core/print_api.php. There are also two changes:

if( config_get( 'allow_inline_attachment_rendering' ) == ON ) {
    # This variable from 1.2.1 before fix 11952 and about 12313 issues:
    $t_href_start = "<a href=\"file_download.php?file_id={$t_attachment['id']}&type=bug\">";
} else {
    # 1.2.2 security fix 11952 and about 12313 issues:
    $t_href_start = "<a href=\"${t_attachment['download_url']}\">";
}

if( config_get( 'allow_inline_attachment_rendering' ) == ON ) {
    # This echo from 1.2.1 before fix 11952 and about 12313 issues:
    echo "\n
$t_href_start<img alt=\"$t_title\" $t_preview_style src=\"file_download.php?file_id={$t_attachment['id']}&type=bug\" />$t_href_end";
} else {
    # 1.2.2 security fix 11952 and about 12313 issues:
    $t_image_url = $t_attachment['download_url'] . '&show_inline=1' . form_security_param( 'file_show_inline' );
    echo "\n
$t_href_start<img alt=\"$t_title\" $t_preview_style src=\"$t_image_url\" />$t_href_end";
}

If somebody may help me to do diff files against 1.2.3 version, I'll upload this patch. Btw, I've upload whole fixed files.

EDIT (dregad): fix markdown

Avg00r

Avg00r

2010-10-04 09:44

reporter  

mantis123_patch_12313.zip (20,341 bytes)
tcowin

tcowin

2011-08-09 12:18

reporter   ~0029417

This is a useful addition, as we are using this in a closed environment, and usability is certainly key -- outweighing those security concerns. This works fine on 1.2.6. It would be great if this could be provided as a standard option in the codebase? Is there a way we can fold this in? Or is this a 'wontfix'?

frankyb

frankyb

2011-10-04 07:15

reporter   ~0029910

I completely second tcowin, in the company its a wish from most users to have the images displayed in the browser instead of being downloaded. A switch (display attachment secure / convenient) would be very nice. If this is a nofix, I will change my patchfiles to represent this feature on the company servers, but having this patch already in official tree would be so nice.

cas

cas

2011-10-04 10:40

reporter   ~0029911

I also support this, we already have a zillion switches, we can do with one more to improve usability in a closed environment.

brahms

brahms

2011-12-30 02:10

reporter   ~0030734

I second vtheiding, tcowin,frankyb, ..., our Mantis is only used internally, we prefer usability.

$g_inline_file_exts:
change the default in config_defaults_inc.php to empty and to add a note about the possible security risks that setting this variable implies.

thanks.

atrol

atrol

2013-04-27 18:31

developer   ~0036702

Removed assignment. dhx will not contribute to this issue in near future.

LeapAhead

LeapAhead

2014-07-24 06:39

reporter   ~0040967

Any news on this? We add a screenshot to almost every bug report (with bug shooting, great tool BTW). Having to download all these screenshots first before you can view them in full screen is nerve wracking! We don't want to hot-fix as proposed by Avg00r to be able to update. Could this change be introduced to the official code?

atrol

atrol

2014-07-24 14:52

developer   ~0040972

New configuration options will hardly be added to version 1.2.
Maybe someone sends a Pull Request on the Github repository [1] for master branch (version 1.3.x)

Until such an option is available: What about dealing with it the way dhx wrote

Avg00r: I suggest enabling inline image previews as this is a more secure way of showing image attachments in the browser

[1] https://github.com/mantisbt/mantisbt

LeapAhead

LeapAhead

2014-07-25 04:01

reporter   ~0040978

our screenshots are normally of websites. In the inline preview they are too small to see comments or markers. But the preview should not be bigger than it is, because with 4-5 screenshots you'd have to scroll forever.

annep

annep

2014-08-28 10:24

reporter   ~0041137

Same for us as for LeapAhead, cas, frankyb, tcowin, ...
It would be great if this would be configurable again.

david.forgianni

david.forgianni

2014-10-02 09:15

reporter   ~0041332

This issue also affects the organization I work for, adding a note to ensure notification of change to this issue.

Can anyone confirm whether the attached patch works for the timebeing, and that I can roll that patch back if i need to upgrade in the future? Thanks!

rattkin

rattkin

2017-05-23 05:20

reporter   ~0056944

please, really allow reverting to old "unsecure" behavior.
We just need to be able to view the images and pdf files in browser directly.

The security benefit of this PITA seems dubious - the attachment is downloaded and then run anyway, there are just several more unnecessary clicks.

some -if not most- teams have a limited set of trusted submitters.

Thank you for consideration.

dregad

dregad

2017-05-23 08:33

developer   ~0056948

This works just fine in 2.x releases as far as I can tell. See 0022588:0056700 for example. What more do you want ?

rattkin

rattkin

2017-05-23 08:42

reporter   ~0056949

upload works fine, thank you.
my beef is with the attachment download instead of direct view, as discussed in this bug.
Live preview is not sufficent for large screenshots, not to mention pdf files.
Just updated to 2.4 recently.

dregad

dregad

2017-05-23 09:09

developer   ~0056950

upload works fine, thank you

Sorry if I wasn't clear; I only meant 0022588:0056700 as an example of viewing image attachments inline.

Live preview is not sufficient for large screenshots

If you click on the inline image, it will open a new tab with it, which I understand is what was initially requested here.

not to mention pdf files.

PDF's are indeed not supported currently, but that's not part of this issue's original requirement. AFAIK previewing a PDF's contents requires use of an external library to generate a thumbnail image which I think is beyond this issue's scope.

siouxzieb

siouxzieb

2017-06-08 17:24

reporter   ~0057046

I am confused by this thread, and by the last example of the image attachment opening in a new tab, as that's not the behavior I'm seeing at all.

I've just updated Mantis to the latest version, haven't used it for a couple years. I increased the allowable size to be able to see the image inline which works fine. But I'm still not able to click to see the full-sized image in the browser, but am instead forced to download via download dialog. I AM able to click to see a PDF, though I think that may just be the way I have Chrome configured. I've created a project here: http://qa-testing.dev.wilsonrms.com/view.php?id=700 to illustrate the issue. It's view status is public, but I'm not sure if that's enough to provide access without a login, as I've never shared anything outside my team.

As others have said, this is really like hitting a wall in the bug review process. If the image is a full-window grab w/markup, the inline preview is way too small to be useful. The opening-in-another-tab thing is fine, but how was that accomplished?

siouxzieb

siouxzieb

2017-06-19 12:44

reporter   ~0057100

Anyone? Anyone? How-to? Little help?

rattkin

rattkin

2017-06-20 02:37

reporter   ~0057102

[quote]AFAIK previewing a PDF's contents requires use of an external library to generate a thumbnail image [/quote]

Generating a thumbnail image is not necesary. All modern browsers have a pdf reader built in. Maybe this is a case of misunderstanding?

The same goes for images. Problem is just that browser is downloading files instead of viewing them. Maybe it's a mime-type setting?

siouxzieb

siouxzieb

2017-06-20 10:36

reporter   ~0057106

It's not really a matter of the browser being capable of reading any particular format. In my case, I view PDFs with a plugin that allows me additional options, which in this case is working fine/as predicted. PDFs aren't the issue. Any browser—modern or not—should render a jpg. But instead, clicking on the image results in either an automatic download to disk, or a dialog offering one. If a user has uploaded a jpg, there's just no reason why I shouldn't be able to open that directly in the browser vs. littering my desktop with yet another file that I simply need to see, not download.

It's the php that's forcing this:
<img src="file_download.php?file_id=278&type=bug&show_inline=1&file_show_inline_token=20170620-CsluvP4ZN2k6U372rZf-97Vn5I75hHu" alt="" style="border: 0; max-height:250px;">

...And the PHP is written into the web app code, obviously, so it seems like Mantis devs could implement the file attachment differently, or at least allow an option for users who are not concerned about any potential security issues.

I added a 'guest' user to my Mantis for reference: http://qa-testing.dev.wilsonrms.com/view.php?id=700

Very much appreciate any input/suggestions. This is really a sticking point from me as I gear up to put MantisBT back in use.

Thanks!

siouxzieb

siouxzieb

2017-06-28 12:46

reporter   ~0057147

So I applied the changes are described above by Avg00r, despite that entry being 3 years old, and since I don't seem to be getting anywhere with my query here on this thread. This resulted in no change that I can perceive. I had hopped the tweaks would contravene the .php, but no such luck. Can anyone add anything of value to this conversation?

Thanks in advance,
Suzanne

rattkin

rattkin

2017-06-30 04:09

reporter   ~0057151

Found it!
Here on www.mantisbt.org/bugs/ images in bugnotes are displayed in browser, but on our private wiki, download dialog is presented by the browser.
Reason: content disposition header is set to "attachment" on my server.

there is a problem in file_download.php
I see a $t_mime_force_inline array and i think the point of all this is to force images and pdf files inline (YAY!), but for some reason, on our server it doesn't work. Maybe other people have this problem too.
In my case $t_content_type is 'image/jpeg'
on line 201 there is some nonsense $t_mime_type = substr( $t_content_type, 0, strpos( $t_content_type, ';' ) );
so that $t_mime_type is now =''

I think it is asumed, that $t_content_type should always have a ';' present, that's maybe wrong.

i'm not good enough programmer to submit a patch, what can i do?

rattkin

rattkin

2017-06-30 04:24

reporter   ~0057152

ok sumbited pull request https://github.com/mantisbt/mantisbt/pull/1125
hope i didn't kill anyone

atrol

atrol

2017-06-30 08:38

developer   ~0057153

Last edited: 2017-06-30 08:39

View 2 revisions

I think it is asumed, that $t_content_type should always have a ';' present, that's maybe wrong.

It seems that @vboctor thought there is always a ; in content type to separate charset
According 0022543:0056203 content type is something like text/html; charset=us-ascii

dregad

dregad

2017-06-30 10:05

developer   ~0057154

I think it is asumed, that $t_content_type should always have a ';' present, that's maybe wrong.
It seems that @vboctor thought there is always a ; in content type to separate charset

As per RFC , the parameter bit is optional, so @rattkin is right, the code in download.php is incorrect.

Related Changesets

MantisBT: master 54929f3b

2017-08-02 08:18:18

dregad

Details Diff
Fix inline viewing of image attachments

The code extracting the MIME type from the content was incorrect,
assuming that a semi-colon would always be present but it's not always
the case.

This resulted in MIME type being empty, which in turn made the browser
download the file instead of displaying the image inline when the web
server's content disposition header is set to "attachment".

Jan Müller's original patch [1] was replaced by more efficient code.

Fixes 0012313

[1] https://github.com/mantisbt/mantisbt/pull/1125
mod - file_download.php Diff File

Issue History

Date Modified Username Field Change
2010-09-01 09:18 Avg00r New Issue
2010-09-01 10:34 atrol Note Added: 0026540
2010-09-01 19:57 dhx Relationship added related to 0011952
2010-09-01 20:03 dhx Note Added: 0026545
2010-09-01 20:03 dhx Status new => resolved
2010-09-01 20:03 dhx Resolution open => won't fix
2010-09-01 20:03 dhx Assigned To => dhx
2010-09-02 07:38 Avg00r Note Added: 0026551
2010-09-02 14:54 Avg00r Note Edited: 0026551 View Revisions
2010-09-02 14:54 Avg00r Status resolved => feedback
2010-09-02 14:54 Avg00r Resolution won't fix => reopened
2010-09-09 07:12 wmadmin Note Added: 0026642
2010-09-12 06:17 vtheiding Note Added: 0026677
2010-10-01 13:22 hanoii Note Added: 0026931
2010-10-04 09:41 Avg00r Note Added: 0026943
2010-10-04 09:41 Avg00r Status feedback => assigned
2010-10-04 09:42 Avg00r Note Edited: 0026943 View Revisions
2010-10-04 09:44 Avg00r File Added: mantis123_patch_12313.zip
2011-08-09 12:18 tcowin Note Added: 0029417
2011-10-04 07:15 frankyb Note Added: 0029910
2011-10-04 10:40 cas Note Added: 0029911
2011-12-29 02:50 rombert Relationship added has duplicate 0013700
2011-12-30 02:10 brahms Note Added: 0030734
2012-09-20 17:04 atrol Relationship added has duplicate 0014722
2013-03-26 05:15 atrol Relationship added has duplicate 0015683
2013-04-27 18:31 atrol Note Added: 0036702
2013-04-27 18:31 atrol Assigned To dhx =>
2013-04-27 18:31 atrol Status assigned => acknowledged
2014-07-24 06:39 LeapAhead Note Added: 0040967
2014-07-24 14:52 atrol Note Added: 0040972
2014-07-25 04:01 LeapAhead Note Added: 0040978
2014-08-28 10:24 annep Note Added: 0041137
2014-10-02 09:15 david.forgianni Note Added: 0041332
2017-03-20 04:29 atrol Relationship added has duplicate 0022543
2017-05-23 05:20 rattkin Note Added: 0056944
2017-05-23 08:09 dregad Note Edited: 0026943 View Revisions
2017-05-23 08:33 dregad Note Added: 0056948
2017-05-23 08:42 rattkin Note Added: 0056949
2017-05-23 09:09 dregad Note Added: 0056950
2017-06-08 17:24 siouxzieb Note Added: 0057046
2017-06-19 12:44 siouxzieb Note Added: 0057100
2017-06-20 02:37 rattkin Note Added: 0057102
2017-06-20 10:36 siouxzieb Note Added: 0057106
2017-06-28 12:46 siouxzieb Note Added: 0057147
2017-06-30 04:09 rattkin Note Added: 0057151
2017-06-30 04:24 rattkin Note Added: 0057152
2017-06-30 08:38 atrol Note Added: 0057153
2017-06-30 08:39 atrol Note Edited: 0057153 View Revisions
2017-06-30 10:05 dregad Note Added: 0057154
2017-08-02 08:33 dregad Changeset attached => MantisBT master 54929f3b
2017-08-02 08:33 dregad Assigned To => dregad
2017-08-02 08:33 dregad Status acknowledged => resolved
2017-08-02 08:33 dregad Resolution reopened => fixed
2017-08-02 08:33 dregad Fixed in Version => 2.6.0
2017-08-02 08:34 dregad Target Version => 2.6.0
2017-09-03 18:41 vboctor Status resolved => closed