auto-corrected, but so many tweaks after to gemspecs it may as well
have been manual
Change-Id: I69aeb6e216894462d6d893ed4c123aa9898fc72f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/278516
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
to get its no-longer-dependency on nokogumbo
the major version bump was only for dropping ruby 2.5
Change-Id: I5a03053e8c3f800a39bcaa384fb5003fa489a2e1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/277905
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
fixes MAT-346
flag=none
Test plan 1:
- Navigate to a RCE instance.
- Set HTML content using flexbox properties
Example: flex-direction, flex-wrap, flex-flow, etc.
- Save content.
- Verify that the content was saved with the flexbox
properties.
Test plan 2:
- Navigate to a RCE instance.
- Set HTML content using grid properties
Example: grid-row, grid-column, grid-template, etc.
- Save content.
- Verify that the content was saved with the grid
properties.
Change-Id: Iff2a57ad43d8526c5f477e0575e188aaa76b4f9f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/272776
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Gary Mei <gmei@instructure.com>
QA-Review: Gary Mei <gmei@instructure.com>
Product-Review: Juan Chavez <juan.chavez@instructure.com>
closes MAT-89
Test Plan
- Test and verify that <track>, <picture>, <map>, <area>, <ruby>,
and <blockquote> work.
- Test and verify that the CSS properties border-radius, grid, and
flex work.
Change-Id: Ie9cdd2fc01bcd8d703afa784c2aa0e8ff136dcc7
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/266575
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
QA-Review: Ed Schiebel <eschiebel@instructure.com>
Product-Review: David Lyons <lyons@instructure.com>
closes MAT-42
flag=new_math_equation_handling
test plan:
This is not easy to test because you really should trigger whatever MathJax does. You could copy/paste
some bad HTML from a prod quiz into the RCE html editor, or use something like this:
<div style="border: 1px dashed red;">
<span style="clip: rect(1e+07em, -9.999e+06em, -1e+07em, -999.997em);">this is a clip</span>
<span style="display: inline-block; overflow: hidden; vertical-align: -1.17e+05em;">this is a vertical-align</span>
</div>
but ^that is what I used to test, so maybe you should try something else.
- have some bad CSS in your RCE html, AND some math so MathJax is loaded
> expect the crazy clip and vertical-align css attributes are stripped
from the offending elements.
Change-Id: Icae13dc8656d2f9918c2b5315bd10089010a8029
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/262868
Reviewed-by: Nate Armstrong <narmstrong@instructure.com>
QA-Review: Nate Armstrong <narmstrong@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
because of nokogiri, we rely on multi-platform gem caching, which
only works right in bundler 2.2
Change-Id: Id207278946e849abec3418807c71e4650506a0cf
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/259142
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
[fsc-max-nodes=12]
[fsc-timeout=60]
* switch lots of parsing to Nokogumbo to keep things consistent
* deep CSS sanitization is now built in, and with a proper parser (meaning
we can drop our code to do it, and adjust some specs to account for things
that _are_ valid)
lots of changes because gumbo parsing<->serialization cycle is slightly different:
* better job preserving original whitespace
* literal non-breaking space characters are converted to entities
* <p> tags aren't inserted for the heck of it
* several _other_ entities are unnecessary, and output as literal characters
* some elements no longer have a closing tag
Change-Id: I7c5e36cbd04b8a05f64c9e0af00868dd6b00f4ce
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/256444
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jacob Burroughs <jburroughs@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
Change-Id: Ib49bc8939cf1706e758429e531a87c57d0231a37
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/251156
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Test Plan:
Let's make sure existing sandbox attributes are properly removed.
Change a page to have the following content:
<iframe src="https://docs.google.com/document/d/1054I9jXtjHmSwFUuj60YsyQ-DloZpORzBffO3KDdv2k" width="100%" height="600" sandbox="allow-scripts allow-forms allow-same-origin"></iframe>
Save
Clicking the 'Request Access' link should open up a confirm modal.
Don't see the error message in the devtools console
flag=none
Fixes CNVS-49914
Change-Id: I261e428185c01f2800c7a89131da2161b00e16ae
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/244965
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Peyton Craighill <pcraighill@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Reviewed-by: Ed Schiebel <eschiebel@instructure.com>
This updates canvas_sanitize gem to allow the skype: protocol in
links in the RCE.
Fixes LS-248
flag = rce_enhancements
Test plan:
- Open RCE (fixes for new and old RCE)
- Switch to HTML editor
- Add <a href="skype:whatever?call">Call whatever</a>
- Save
- Click the link
- Expect browser to open Skype (may present warning first,
depending on browser)
See http://enarion.net/programming/c-sharp/skype/html-call/ for
an explanation of the skype protocol.
Note: In both the new and old RCE, adding a skype: link via the
rich content mode, will only work if the link is skype://username
(with the slashes) - skype:username will not work, but is being
addressed in the new RCE in LS-1232.
Change-Id: I03cc1be046cf79231538bd9ae7c32fa41ba62cd1
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243978
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Clint Furse <cfurse@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Jackson Howe <jackson.howe@instructure.com>
The iframe sandboxing made it so media uploads
didn't properly render after being uploaded. This
is obviously a problem, so this patchset adds
the allow-same-origin sandbox permission to
all iframes.
This effectively reverts LS-743, so that ticket
was reopened so it can be addressed at a
later date.
This will need to be warm fixed.
Test Plan:
Upload media with the new RCE.
Save it.
The media should load correctly on the page.
Refs LS-743
Fixes LS-1260
Change-Id: I130af41a369d19ec02abf18682569a88d0a18f77
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243902
QA-Review: Robin Kuss <rkuss@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
Product-Review: Alex Anderson <raanderson@instructure.com>
Since files are hosted on the same domain as the Canvas
instance, it has access to localStorage, cookies, and
autofill passwords. This can be used to capture user data
for nefarious purposes through XSS.
This adds a `sandbox="allow-scripts allow-forms"` attribute
to the preview iframe to disallow same-origin behaviors.
This patchset also applies sandbox rules to iframes
embedded in user-generated content. This is done
with the onChange handler.
Test Plan:
This will be a little involved, so bear with me.
Log out of Canvas. On the login page, save the raw HTML
to your computer as a .html file. In Chrome this is done
with the "Save Page As..." option.
This will create an HTML page that copies the Canvas Login.
Then, go to the Files page for a course and upload this
HTML file.
Preview the HTML file. If you have autofill passwords enabled,
the username and password fields should not be filled in.
If you don't have autofill passwords enabled, that's okay.
Instead, open the preview for the HTML file.
Then open the dev tools. In the console, change your
JavaScript context from "top" to "preview". Then, try
to execute the following JavaScript (sans quotes):
`localStorage`
If this throws an error, it's working as expected.
If it logs the `localStorage` object, then the frame
still has access to the top origin.
To test user-generated content, navigate to an RCE,
and use the Raw HTML Editor to add an iframe. Close
the Raw HTML editor, make a change in the editor,
then and open the HTML editor again. The sandbox
attributes should have been added to your iframe.
flag=none
Refs LS-743
Change-Id: I9fe5660d8592423e6435d2b7ed6804b05c74200a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243228
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jackson Howe <jackson.howe@instructure.com>
QA-Review: Robin Kuss <rkuss@instructure.com>
Product-Review: Alex Anderson <raanderson@instructure.com>
Closes PLAT-5638
flag=none
Test Plan:
- Navigate to an RCE and switch to the html view
- attempt to add an anchor tag that uses the "tel"
protocol in the href attribute
- Switch back away from the HTML editor and verify
the link's href attribute remains correct
Change-Id: Ib0412001163f52bf5848297232946717c85d01be
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/231231
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Alex Slaughter <aslaughter@instructure.com>
Reviewed-by: Evan Battaglia <ebattaglia@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Weston Dransfield <wdransfield@instructure.com>
the allowed list of elements removed by canvas should never get
past the RCE
closes LA-526
flag=rce_enhancements
test plan:
- using the html view, create some content in the RCE with
disallowed elements (like main, form, select, button, script)
- switch out of html view and back
> expect the disallowed elements to have been removed, changed
to something else, or html encoded
- not sure how to test this, but canvas should never have to
remove any elements from content created by the RCE
Change-Id: I89c1149e8b24451fc59e9a879b13e549504e1704
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/224717
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jon Willesen <jonw+gerrit@instructure.com>
Product-Review: Ed Schiebel <eschiebel@instructure.com>
test plan:
- before checking out this patch set, put the following text in
a wiki page:
<a class="external_url_link" href="#"
data-item-href="javascript:alert('hi')">XSS</a>
(note that reproducing the issue depends on a race condition;
the page text must be present before the domready callbacks in
context_modules.js run)
- check out the patch set and note that clicking the link
takes you to about:blank instead of running javascript
- use the wiki pages API to try to inject the text from step 1
into a page. the data-item-href attribute should be stripped
fixes LA-499
flag=none
Change-Id: I1ce377c9a386332ae336fc4ffe7a2bf257364a11
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/224051
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Jon Willesen <jonw+gerrit@instructure.com>
QA-Review: Anju Reddy <areddy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Fixes: CORE-2938 CORE-2937 CORE-2935 CORE-2933
We have a bunch of stuff in our code where we use data-whatever
attributes to tell our javascript what to do. and a few of those treat
whatever is in data-whatever as trusted html. The problem is that users
can save whatever data-* attributes in the content they save
in a rich text editor. So if they mimic the same classnames or ids as
elements we trust, they can exploit us.
The fix is to still allow data-* attributes EXCEPT the ones we treat
as html. By filtering those out, we can treat them as trusted html.
Refs: SEC-2166 ADMIN-2376
This partially reverts ADMIN-2376 because instead of having to load all
tinyMCE to sanitize it on the client, if we treat it the same as all of
these other ones, we can just count on data-html-tooltip-title as coming
from us.
NOTE: in the future:
1. Don’t treat the Dom as the source of truth.
2. If you do, use an attribute like x-canvaslms-trusted-whatever
because that will not be allowed through in our html sanitizer
so we can know if that attribute is ever there on an element,
we put it there and not a user or hacker.
Test plan:
* Look at the repro steps on any of the attached jiras.
* You should not be able to reproduce it
* there is an automated test fixture for each one that has been added
To spec/fixtures/xss/
Change-Id: Idf77ac6d80518bfe02445f94942ac5f6802772dc
Reviewed-on: https://gerrit.instructure.com/194850
Tested-by: Jenkins
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
QA-Review: Clay Diffrient <cdiffrient@instructure.com>
Product-Review: Clay Diffrient <cdiffrient@instructure.com>
clsoes CORE-1883
test plan:
- edit a wiki page
- switch to html editor
- add <mark>test</mark>
- save the page
- the mark tag should not be stripped out
Change-Id: Ic79b5554e0254b54797f78d5d4ab7e5e6c8281bf
Reviewed-on: https://gerrit.instructure.com/164510
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
QA-Review: Brent Burgoyne <bburgoyne@instructure.com>
Fixes: CORE-1556
* go to an rich content editor
* add this markup with a “longdesc” attribute to the page
<img
src=“https://placekitten.com/200/300”
longdesc=“http://www.example.com/“
/>
* save it
* reload the page
* make sure the longdesc attribute is still there
Change-Id: I8c57668e3c257ae186a7ea94c40dceb87835ca33
Reviewed-on: https://gerrit.instructure.com/155020
Tested-by: Jenkins
Reviewed-by: Brent Burgoyne <bburgoyne@instructure.com>
QA-Review: Nathan Rogowski <nathan@instructure.com>
Product-Review: Ryan Shaw <ryan@instructure.com>
this is only a temporary fix, until we have an account setting to
manage a whitelist of domains that allow the allow attribute.
fixes CORE-1157
refs CORE-1241
test plan:
- create/edit a wiki page
- click the HTML Editor link
- add an iframe with allow="microphone; camera"
- save the wiki page, and edit again
- view the html, it should still have the allow attribute
Change-Id: Ia1bc6507e830a6a1e91060fee7016f444f850ad7
Reviewed-on: https://gerrit.instructure.com/145695
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Brent Burgoyne <bburgoyne@instructure.com>
fixes gh-1159
test plan:
- try to create an audio tag with the controls attribute
- it should persist across save
Change-Id: I84ec4ce6fecacdbaff91d8d53e8dde4b7ab5c736
Reviewed-on: https://gerrit.instructure.com/133028
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
test plan:
* in a blueprint course create an assignment, discussion and
a quiz all with a link to a file in their description
* lock content for all of them
* sync to a child course
* in the child course, should be able to edit dates without
getting an error
close #CNVS-38520
Change-Id: I0eb7331094871d8e2698ee1357fa2c0c16f3bd46
Reviewed-on: https://gerrit.instructure.com/122384
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
QA-Review: David Mirabile <dmirabile-c@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
test plan:
see coverage in previous rspec/selenium builds, same numbers as before
Change-Id: I331bf5102914da00a5d350f32b74b4bc9d49c5f8
Reviewed-on: https://gerrit.instructure.com/106895
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
fixes CNVS-35749
test plan:
- make sure you can create a video tag in the RCE with the 'playsinline'
attribute
- general regression test of embeding video in the RCE (directly, from
a file, any other ways to get a video tag)
Change-Id: Ib7460ea8175d768e97b9b49acea67b8199bc4edf
Reviewed-on: https://gerrit.instructure.com/105709
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
closes CNVS-34574
test plan:
- try to create rich content with the abbr tag
- it should work
Change-Id: I28ae285524fb671fc69709732976df2edd0d5d6d
Reviewed-on: https://gerrit.instructure.com/101164
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <ryan@instructure.com>
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
fixes CNVS-34785
test plan:
- try to create a math tag with a valid link, and with a stored script
- the valid link should work, but the stored script should be stripped
Change-Id: I1bec604d673d5ece23b238e1d81a292baa68fa6f
Reviewed-on: https://gerrit.instructure.com/101202
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
as of this commit, all canvas gems should be on rspec 3.5, and pass
without deprecation warnings.
closes CNVS-34040
test plan: specs should pass without deprecation warnings
Change-Id: I556b1a4a5aeb791c6ddd50ee35b51c513e025019
Reviewed-on: https://gerrit.instructure.com/98414
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Tested-by: Jenkins
fixes CNVS-30404
test plan:
- add a video/audio with a track tag in the html editor
- toggle back and forth between rich text and html and make sure it
stays
- save and edit and make sure it stays
Change-Id: I18418ef98ed2c03d1a4901a4bf15cf82474c6131
Reviewed-on: https://gerrit.instructure.com/85159
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Putnam <jeremyp@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
Fixes CNVS-28473
Test plan:
- Visit a page with an editor
- Switch to the HTML view.
- Insert MathML.
- Switch to and from the RCE editor and HTML editor.
- Verify that the MathML remains unchanged.
- Insure that it is not removed or otherwise redacted on save.
Change-Id: I46d55505518f0427fa342eff175a58bdcb0c5284
Reviewed-on: https://gerrit.instructure.com/79588
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Aaron Cannon <acannon@instructure.com>
QA-Review: Pierce Arner <pierce@instructure.com>
This PR allows data-* attributes in HTML to pass the santization process.
For example, this allows integration with Embedly if the content includes
HTML such as...
<a class="embedly-card" href="http://www.instructure.com/" data-card-controls="0" data-card-type="article">Instructure</a>
...and the appropriate JS is loaded.
Test Plan:
- Edit a wiki page using the HTML editor and enter custom HTML5 code that includes data attributes
- Verify the attributes exist in the HTML when the page is rendered
Note: This is minor version upgrade to sanitize. It does not upgrade to 3.x or 4.x
which could introduce more changes.
rely on test_all_gems.sh to output header and trailer,
and use `set -e` in each test.sh to simplify passing
through errors
Change-Id: I3ba724ad2539ddfe31195394c43f646acfc73920
Reviewed-on: https://gerrit.instructure.com/70469
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
* add test runner for broadcast_policy and fix a couple of tests that
had been broken by accident
* add test runner for canvas_quiz_statistics and add a reference to
iconv to it's Gemfile to allow tests to run in ruby 2
* add a test runner for canvas_sanitize
* remove leftover cruft from running gem builds in both rails 2 and 3
Change-Id: Iab2a0986f277a82c096e1fff2dab1d3a55b91733
Reviewed-on: https://gerrit.instructure.com/54518
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Cody Cutrer