View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
19197 | Bug reports | Question editor | public | 2023-10-25 23:05 | 2023-10-27 03:41 |
Reporter | diana_was_here | Assigned To | |||
Priority | none | Severity | minor | ||
Status | new | Resolution | open | ||
Product Version | 6.3.x | ||||
Summary | 19197: Sandbox attribute added to iframe in question editor | ||||
Description | After recent limesurvey updates, I became unable to use <iframe>s in the survey questions. The iframes are displayed but interaction is disabled. I realised this is because whenever I include an iframe, limesurvey automatically adds the sandbox attribute: sandbox="allow-scripts allow-same-origin" . And despite the exception in the sandbox (allow-scrips), no scripts are permitted to run in the iframe. The most frustrating aspect is that whenever I try to remove the sandbox attribute in the Source editor, this is automatically added back to the code. I eventually managed to get around this issue by disabling the question editor in my account preferences and using the sourcecode editor instead. By using the sourcecode editor, I am finally able to remove the sandbox attribute that kept being added to my iframes, thanks to the very helpful limesurvey community: https://forums.limesurvey.org/forum/installation-a-update-issues/142813-remove-sandbox-attribute-from-iframes . I attach two images to illustrate: Further to this, after the addition of the sandbox attribute, scripts in the iframe won't work. In the test code, there is a button triggering an alert. The iframe contains that same code of the button triggering the alert. What I see when I run the code with the 'sandbox' addition is the two buttons, but the alert is only triggered by the button directly added to the html code, but not the iframe one. | ||||
Steps To Reproduce | Steps to reproduceGo to the default question editor. Click on Source. Include an <iframe> element. Expected resultI expect to be able to interact with the page displayed in the iframe, i.e. for scripts to run, forms to be submitted etc. Actual resultThe Source editor adds the sandbox attribute and all interaction is disabled. | ||||
Tags | No tags attached. | ||||
Attached Files | |||||
Bug heat | 8 | ||||
Complete LimeSurvey version number (& build) | 6.3.0+231016 | ||||
I will donate to the project if issue is resolved | No | ||||
Browser | tested on Firefox + Chrome | ||||
Database type & version | not sure | ||||
Server OS (if known) | |||||
Webserver software & version (if known) | |||||
PHP Version | 7.4. | ||||
https://github.com/LimeSurvey/LimeSurvey/blob/590c1ac76e2da298d01c34ac4e9fe7f9138e343d/assets/packages/ckeditor/config.js#L341 is the responsible line of code. Added in this commit: https://github.com/LimeSurvey/LimeSurvey/commit/8569f7e913c1c849659c3e5b75de096cebca4afb to fix the editor adding sandbox="" (most restrictive value is empty string: https://www.w3schools.com/tags/att_iframe_sandbox.asp) So my guess is the fix was just to explicitly state the sandbox attribute values rather than letting the editor decide. The issue for the mentioned commit is on jira... Need to check browser behavior. So yeah workaround is disable the editor. |
|
@diana_was here anything in browser console? js is not working at all? or partial? |
|
from: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/iframe allow-scripts allows js, but no pop up window allow-popups/allow-popups-to-escape-sandbox is needed for this do we have way for user to extend editor config? |
|
can you try enabling editor, and add allow-popups/allow-popups-to-escape-sandbox (depending on needs) to sandbox attribute value and see if your js works as expected then? |
|
anyways, don't think its a bug just misconfigured |
|
also thinking about this, not sure i agree with having a default value at all... just in general, lots of threats to users evolve over time to exploit iframe use. so, browsers provide a way to increase user security when using iframes. limesurvey users may use iframe. shouldn't each user decide how much security their particular use of iframe needs rather than disabling protections by default because some users don't care to understand/configure the setting? otherwise, we just pass on risk to all the survey respondents. (also users of the software) this is more like default allow than default deny policy |
|
I see this as a regression. In 5.x the editor did not set a sandbox attribute. I think it should be left to the user to set element attributes. Can this be defined in the editor config settings? |
|
per mdn allow scripts and allow same origin (LS current default settings) is no more secure than running with no sandbox at all. so if you see this issue as a regression, why don't we just remove the attribute entirely (revert to pre 4.21 editor behavior)? ckeditor docs: https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-iframe_attributes simple enough change. on https://github.com/LimeSurvey/LimeSurvey/blob/590c1ac76e2da298d01c34ac4e9fe7f9138e343d/assets/packages/ckeditor/config.js#L340 just set to an empty object my feeling is we should leave default sandbox="" (most restrictive) and put a note in the manual for question editing regarding iframes, and link to mdn docs. this defaults to most secure and if a user needs less security, they can choose which exceptions to apply for their use case and add them to the attribute. how common is iframe use anyways? i think this is akin to disabling the incoming firewall just because a user needs one port open and doesn't know how |
|
Date Modified | Username | Field | Change |
---|---|---|---|
2023-10-25 23:05 | diana_was_here | New Issue | |
2023-10-25 23:05 | diana_was_here | File Added: enter code edited.png | |
2023-10-25 23:05 | diana_was_here | File Added: enter code.png | |
2023-10-25 23:33 | mfavetti | Note Added: 78017 | |
2023-10-25 23:33 | mfavetti | Bug heat | 0 => 2 |
2023-10-25 23:35 | mfavetti | Note Added: 78018 | |
2023-10-25 23:38 | mfavetti | Note Added: 78019 | |
2023-10-25 23:39 | mfavetti | Note Added: 78020 | |
2023-10-25 23:39 | mfavetti | Note Added: 78021 | |
2023-10-25 23:55 | mfavetti | Note Added: 78022 | |
2023-10-25 23:56 | mfavetti | Note Edited: 78022 | |
2023-10-26 00:44 | mfavetti | Issue Monitored: mfavetti | |
2023-10-26 00:44 | mfavetti | Bug heat | 2 => 4 |
2023-10-26 08:02 | DenisChenu | Issue Monitored: DenisChenu | |
2023-10-26 08:02 | DenisChenu | Bug heat | 4 => 6 |
2023-10-26 12:01 | tpartner | Note Added: 78038 | |
2023-10-26 12:01 | tpartner | Bug heat | 6 => 8 |
2023-10-27 03:41 | mfavetti | Note Added: 78069 |