Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Thumbnail generator should also make sure that the file is an image #5485

Closed
alecpl opened this issue Oct 21, 2016 · 3 comments
Closed

Thumbnail generator should also make sure that the file is an image #5485

alecpl opened this issue Oct 21, 2016 · 3 comments
Assignees
@alecpl
Labels
bug C: Security
Milestone
1.2.3

Comments

@alecpl
Copy link
Member

alecpl commented Oct 21, 2016

As suggested in #4151 (comment) it is possible to bypass security checks by using e.g. swf-file with jpg extension and image/jpeg mimetype. If the image resize fails we should make sure the file is an image before sending it to the browser as we do for inline images.
@alecpl alecpl added this to the 1.2.3 milestone Oct 21, 2016
@rpv-tomsk
Copy link

rpv-tomsk commented Oct 21, 2016 •
edited
Loading

As reporter, I think these security checks by file content are excessive.

The response will be with 'Content-Type: image/xxx' header, so browser must treat content as image and nothing else. Can anybode please explain security hole more detailed? Google keywords and other links are highly welcome. (And, please, do not open #4151 (comment) - there is swf-file inlined disguised as image.)

Propose to discuss this. )

Maybe CSRF protection ($RCMAIL->request_security_check(rcube_utils::INPUT_GET)) should be added there too?

@alecpl
Copy link
Member Author

alecpl commented Oct 21, 2016 •
edited
Loading

#4014 talks about an iframe, maybe with img there's no such issue in browsers, but I don't know.

As for the CSRF check, I'm not sure.

@alecpl alecpl self-assigned this Nov 20, 2016
alecpl added a commit that referenced this issue Nov 20, 2016
@alecpl
Fix missing content check when image resize fails on attachment thumb…
6287227 
…nail generation (#5485)
alecpl added a commit that referenced this issue Nov 20, 2016
@alecpl
Fix missing content check when image resize fails on attachment thumb…
5162b20 
…nail generation (#5485)
@alecpl
Copy link
Member Author

alecpl commented Nov 20, 2016

Fixed. For other considerations use mailing list.

@alecpl alecpl closed this as completed Nov 20, 2016
ZiBiS added a commit to ZiBiS/roundcubemail that referenced this issue Nov 23, 2016
@ZiBiS
Merge branch 'master' of https://github.com/roundcube/roundcubemail
d462523 
* 'master' of https://github.com/roundcube/roundcubemail: (31 commits)
  Fix write_log() return value when using syslog() and it fails
  Fix alignment of error icon
  Fix _from argument validation
  A better alignment/positioning of icons on widescreen list
  Update changelog
  Revert some style changes to correctly position status icon in threaded messages view of widescreen mode
  Always send columns list as array (unsetting items makes it an object with numeric keys)
  Adjust sizes and spacings in new 3-column layout
  Support hostname and hostname:port in force_https option (roundcube#5511)
  Fix displaying attached images with wrong Content-Type specified (roundcube#5527)
  Fix missing content check when image resize fails on attachment thumbnail generation (roundcube#5485)
  Make sure $prefs property is an array (roundcube#5523)
  Fix storing "empty" values in rcube_cache/rcube_cache_shared (roundcube#5519)
  replace old trac links (roundcube#5514)
  Enigma: Don't log bad-passphrase errors
  Require Crypt_GPG 1.6.0
  Update README with some GnuPG 2.1 support info
  GnuPG 2.1: Fix secret keys export
  write_record() should return boolean value
  Code simplification
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@alecpl alecpl

Labels
bug C: Security
Projects
None yet
Milestone
1.2.3
Development

No branches or pull requests
2 participants
@alecpl @rpv-tomsk