Project

General

Profile

Feature #6814

Allow data uri scheme in security.lib.php

Added by Kevin LEVRON over 6 years ago. Updated almost 6 years ago.

Status:
Feature implemented
Priority:
Normal
Category:
Global / Others / Misc
Target version:
Start date:
08/11/2013
Due date:
% Done:

100%

Estimated time:
Spent time:
Complexity:
Normal
SCRUM pts - complexity:
?

Description

Hello

It should be interesting to allow uri data scheme in security.lib.php :
e.g. some of our teachers paste images (using firefox) in exercices answers, and firefox use data uri scheme to encode these images.

This could be allowed in remove_XSS function, e.g. :

$config->set('URI.AllowedSchemes', array(
    'http' => true,
    'https' => true,
    'mailto' => true,
    'ftp' => true,
    'nntp' => true,
    'news' => true,
    'data' => true,
));

Associated revisions

Revision 506d7e06 (diff)
Added by Julio Montoya about 6 years ago

Allow data uri scheme see #6814

History

#2

Updated by Julio Montoya about 6 years ago

  • Status changed from New to Assigned
  • Assignee set to Yoselyn Castillo

I don't see a problem to adding this.

Hi Yoselyn,

Could you take a look?

#3

Updated by Yoselyn Castillo about 6 years ago

  • Status changed from Assigned to New
  • Assignee deleted (Yoselyn Castillo)
#4

Updated by Yannick Warnier about 6 years ago

Kevin, does that mean that, with this feature, the copy-pasting of images directly from word, for example, into a question, works and is not filtered out?
That would be a GREAT addition!

#5

Updated by Kevin LEVRON about 6 years ago

Well, yes but it is a firefox feature only.

And there are at least two drawbacks :
- this will increase page size since these kind of images are not cached,
- some old browsers don't handle the uri scheme.

#6

Updated by Yannick Warnier about 6 years ago

OK. What I've seen is that copy-pasting tries to transform images into base64 encoding forms of the binary image data and puts them into the corresponding description field (which is then stored into the database), but I haven't focused on any detail (I don't think it was url schemes though). What is it exactly that your feature is supposed to do once you copy-paste something?

#7

Updated by Kevin LEVRON about 6 years ago

Without allowing uri data scheme in remove_XSS, Chamilo will not display these kind of images, src attribute is filtered.

#8

Updated by Yannick Warnier about 6 years ago

  • Category set to Global / Others / Misc
  • Status changed from New to Assigned
  • Assignee set to Julio Montoya

Apparently most browsers now support it, to the exception of IE7 and lower, and "Internet Explorer 8 and above only supports data URIs for images in CSS, <link>, and <img>." (which is fine, as far as I am aware).
Apparently, the Opera browser limits data URIs to around 4100 characters (so it would only work for very small images)

Sources

Julio is taking this one in charge, but in general I agree (the only thing we are checking is if that doesn't introduce some kind of security flaw, but given the fact that HTMLPurifier supports it, I'm relatively comfortable with it.

#9

Updated by Julio Montoya about 6 years ago

  • Status changed from Assigned to Needs testing
  • Assignee deleted (Julio Montoya)
  • % Done changed from 0 to 90

Using 'data' should be fine, htmlpurifier only allows images:

https://github.com/jmontoyaa/chamilo-lms/blob/7a8b90ad77339877fa6460594de818a49c6985da/main/inc/lib/htmlpurifier/library/HTMLPurifier/URIScheme/data.php#L12-L14

public $allowed_types = array(
        // you better write validation code for other types if you
        // decide to allow them
        'image/jpeg' => true,
        'image/gif' => true,
        'image/png' => true,
        );
#11

Updated by Yannick Warnier about 6 years ago

  • Assignee set to Daniel Barreto

I'm OK with all the disadvantages, but I think we should put the link in the changelog.
Assigning to Daniel for a little bit of testing and approving the change.

#12

Updated by Julio Montoya about 6 years ago

Changes in chash (for chamilo 1.10) longtext is required in some fields otherwise the data will be truncated.

https://github.com/chamilo/chash/commit/cd5423055ba27efe5ae2804129b45acf34d6fbcc

#13

Updated by Daniel Barreto about 6 years ago

Testing and have some problems, added some images dragged from a folder to the answers of exercises and they are showing these images when the user see the results of the test but no while submit the test.

EDIT:
The problem was when the user add a image whit drag and drop, the mime type isn't defined. When manually added data type it worked.

#14

Updated by Yannick Warnier almost 6 years ago

  • Status changed from Needs testing to Feature implemented
  • % Done changed from 90 to 100

Most of the tests I've done have worked (I don't remember if any has failed) and support for browsers is good. I'm closing this task. Thanks a million for the suggestion, Kevin!
You've been added to the hall of fame and so has the Université de Pau: https://stable.chamilo.org/documentation/credits.html (let me know if I forgot someone, I'm not good with names).

Also available in: Atom PDF