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.

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

Saving from source dialog executes script tag contents #3470

Open
toshniba opened this issue Sep 20, 2019 · 7 comments
Open

Saving from source dialog executes script tag contents #3470

toshniba opened this issue Sep 20, 2019 · 7 comments
Labels
status:confirmed An issue confirmed by the development team. type:feature A feature request.

Comments

@toshniba
Copy link

Type of report

Bug

Provide detailed reproduction steps

  1. Click on source button
  2. enter <script>alert('poc');</script>
  3. Click on save button

Expected result

Script tag should be removed before saving as it would when changing back from source view before saving.

Actual result

Seems the sanitizer is not executed when saving from source view.

Other details

  • CKEditor version: 4.12.1
@toshniba toshniba added the type:bug A bug. label Sep 20, 2019
@f1ames
Copy link
Contributor

f1ames commented Sep 20, 2019

@toshniba I assume Save button you mentioned is some custom integration because CKEditor 4 doesn't provide one.

The getData() method called when editor is in Source Mode returns it's current content. Since you are able to type anything there (that's its purpose) and it is parsed and converted when switching to WYSIWYG mode, you will get raw data. It works like that by design. Your implementation should only allow saving data when in WYSIWYG mode.

@f1ames f1ames closed this as completed Sep 20, 2019
@f1ames f1ames added the resolution:by-design Described behavior is a part of feature design and is not expected to be changed. label Sep 20, 2019
@toshniba
Copy link
Author

This is definitely an official plugin as stated out on its site:
https://ckeditor.com/cke4/addon/save
Please reopen and classify as a bug then.

@f1ames
Copy link
Contributor

f1ames commented Sep 20, 2019

@toshniba Thank you for clarification, we will re-investigate this issue.

@f1ames f1ames reopened this Sep 20, 2019
@f1ames f1ames added status:pending and removed resolution:by-design Described behavior is a part of feature design and is not expected to be changed. labels Sep 20, 2019
@Dumluregn
Copy link
Contributor

This behaviour can be reproduced using our sample for save plugin.
It still needs to be clarified though if we should filter content in this case or leave it to developers to take care the content they send to server is properly validated.

@spacevoyager78
Copy link

As a temporary workaround, I would cancel the 'save' event and alert the user when in source mode, by checking mode property in 'save' event.
Something like that:

CKEDITOR.instances.editor1.on('save', function(evt) {
	if (evt.editor.mode == 'source') {
		alert('You can't save contents in source mode');
		evt.cancel();
	} else {
		// continue with save
	}
});

@spacevoyager78
Copy link

Or even better, disable toolbar icon for source mode:

CKEDITOR.instances.editor1.on('instanceReady', function(evt) {
	evt.editor.commands.save.modes.source = 0;
}

@Dumluregn Dumluregn added status:confirmed An issue confirmed by the development team. and removed status:pending labels Sep 27, 2019
@Dumluregn
Copy link
Contributor

Although it's not really bug on our side, we think it wouldn't harm to filter editor content in the case you described, so we will treat it as a feature request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:confirmed An issue confirmed by the development team. type:feature A feature request.
Projects
None yet
Development

No branches or pull requests

4 participants