Problem/Motivation

With the latest CKEditor 4.15.1, WYSIWYG API reports: "The version of CKEditor could not be detected." at '/admin/config/content/wysiwyg'

Steps to reproduce

Step 1: Make sure the version is 4.15.1 in libraries/ckeditor.
Step 2: Go to '/admin/config/content/wysiwyg', it shows "The version of CKEditor could not be detected."

Proposed resolution

- Change editors/ckeditor.inc line#35 from "'verified version range' => array('3.0', '4.14.0.8a12b04171')" to "'verified version range' => array('3.0', '4.15.1.1aa21195b')"
- Add the minimum amount of other changes needed to up-rev CKEditor to 4.16.1 and roll a Wysiwyg 7.x-2.7 release from that.

Remaining tasks

- Identify which patches from the Wysiwyg-dev branch must be applied to fix the current Wysiwyg 7.x-2.6 release such that CKEditor 4.16.1 functions correctly.

User interface changes

API changes

Data model changes

Comments

lily.yan created an issue. See original summary.

lily.yan’s picture

StatusFileSize
new595 bytes

The attached patch can fix this issue.

lily.yan’s picture

Status: Active » Needs review
devad’s picture

Title: Update to CKEditor 4.15.1 » Update to CKEditor 4.16.1
Priority: Normal » Critical
Status: Needs review » Needs work

CKEditor 4.16.1 has an important security fix. It is fixed today in D8/9.

Drupal core - Moderately critical - Cross Site Scripting - SA-CORE-2021-003

So, Wysiwyg module for D7 should add support for CKEditor 4.16.1 asap.

And a new tagged release would be nice when this is fixed so that D7 users do not depend on dev release for this security fix.

deadpoet’s picture

StatusFileSize
new644 bytes

Here's a patch to support the latest CKEditor 4.16.1.cae20318d4 (to address the SA-CORE-2021-003 vulnerability).

deadpoet’s picture

Status: Needs work » Needs review
devad’s picture

Status: Needs review » Reviewed & tested by the community

Patch #5 applies cleanly and works for me. Thank you @44sunsets.

Edit: see comment #35.

jamesoakley’s picture

I'm getting "Hunk failed" when I try to apply the patch to 7.x-2.6. I'd rather not move to -dev because I don't know if there are any other changes the maintainers have made that may not be ready for the light of day.

devad’s picture

Comment deleted. Outdated.

joey-santiago’s picture

StatusFileSize
new596 bytes

@JamesOakley
i'm in the same situation. i hope this patch will work for you. Seems to work fine!

jmcf’s picture

#10 appears to work without any problems for me.

devad’s picture

Since patch #10 is for Wysiwyg 7.x-2.6 it is better to hide it so that users do not get confused when testing.

The proper patch for -dev testing (and future commit) here is patch #5.
Patch #10 is for Wysiwyg 7.x-2.6 purpose only.

jamesoakley’s picture

Related issues: +#3216007: Release 7.x-2.7

Thanks - #10 applied nicely to me, so thanks to @joey-santiago for taking the time to reroll it and saving me a job. :-)

jamesoakley’s picture

Status: Reviewed & tested by the community » Needs work

The patch successfully changes the maximum accepted version. This means you can install 4.16.1 and successfully switch text formats over to use that version.

However, having done that, none of my comment / node edit forms displayed the CK editor form. When I reverted back to the version I was using before (which, for me, happens to be 4.9.2) I got the wysiwyg editing back again.

It seems there's some other compatibility issue other than just changing the max allowed version.

nitheesh’s picture

+1 RTBC. #5 cleanly applied to the dev.

jamesoakley’s picture

@nitheesh - does it work though? (See my comment #14)

nitheesh’s picture

@JamesOakley I'm using wysiwyg dev version 7.x-2.6+2-dev and the comment, node edit forms are working fine.

mcdruid’s picture

Status: Needs work » Reviewed & tested by the community

@JamesOakley it'd be great if you were able to test upgrading your site to the latest dev release + the patch, as that'd presumably be what the next release of the module would be based on (especially if a security-fix-only would suffer from regressions as your comments suggest).

jamesoakley’s picture

@mcdruid, I've just tested that, and the dev release + the patch in #5 gave me the same problem.

I need to do some more testing on a clone of the site to see if other modules (like CDN / AdvAgg in particular) could be interfering. I'll report back.

joey-santiago’s picture

@James

thanks for making me realize this. In my case, i had downloaded a custom build of ckeditor that i placed in my libraries folder, so i had to manually download the newer version and ship that one with the new module as well.

i confirm updating the custom build + updating the module to 2.6 with patch in #10 seems to work fine.

Just a heads up: updating the module and patching it might not be enough. To be sure that ckeditor is actually using the version 4.16.1, one should also load a page where the ckeditor is used and make sure the ckeditor.js file has the version 4.16.1 in it :)

jamesoakley’s picture

Status: Reviewed & tested by the community » Needs work

I've now installed 7.x-2.x-dev of this module, applied the patch from #5, and unzipped ckeditor_4.16.1_full.zip into the libraries folder. I've also uninstalled any modules that could conceivably affect the JS on a page - including, but not limited to CDN and all modules / submodules in the AdvAgg package. All caches have been cleared.

The wysiwyg editor does not appear in my comment / node creation pages.

If, with that combination of modules, I simply replace the ckeditor folder in libraries with 4.9.2, the wysiwyg editor resumes normal service.

nloomis’s picture

I have a similar issue. wysiwyg2.6, newest full CKE library, and patch #10 applied. Editor does not appear on any node though.
Inspect Console gives the error "Failed to load resource: the server responded with a status of 404 (Not Found) ckeditor.js:1"

I am at a loss

jamesoakley’s picture

Steps to reproduce

1. Untar Drupal core to the web root
2. Insert database credentials into settings.php
3. Download and untar two modules into sites/all/modules - Libraries and Wysiwyg (-dev release). Apply the patch from #5.
4. Create sites/all/libraries
5. Download ckeditor 4.16.1-full.zip, and unzip in the libraries directory just created.
6. Install Drupal. Set up admin user
7. Create a content type called Basic Page
8. Create a text format called Full HTML that converts line breaks into HTML paragraph tags, but otherwise does not limit the HTML that can be used.
9. Enable the Wysiwyg module. Create a Wysiwyg Profile assigning ckeditor 4.16.1 to the Full HTML text format. Check the boxes to have ckeditor offer you toolbar buttons for bold and italic.
10. Go to create a Basic Page node. Note that there is no wysiwyg editor offered.
11. Delete the ckeditor folder in libraries, and replace with an unzipped copy of ckeditor 4.9.2-full.zip.
12. Go to the Wysiwyg Profiles admin page, and edit and save the profile being used, so that the module stops complaining that it's not got the 4.16.1 it's looking for
13. Go to create a Basic Page node. Note that you are now presented with a wysiwyg editor that has bold and italic buttons.

All of this was done with a freshly installed site, with no other modules, using just Bartik.

awebmanager’s picture

I am confused by SA-CORE-2021-003 issued yesterday as it seems to say that CKEditor should be updated, i.e. the code inside sites/all/libraries, yet it also says that "Users of the CKEditor library via means other than Drupal core should update their 3rd party code (e.g. the WYSIWYG module for Drupal 7)". I have updated CKEditor in libraries across my D7 sites to version 4.16.1.cae20318d4. I have Wysiwyg version 7.x-2.2+46-dev on one site and 7.x-2.5 on another but I don't get any error messages or issues on either site as seems to be reported on this ticket. On the site where I have Wysiwyg 7.x-2.5 installed I do get the message "CKEditor (Expected version: 4.9.2.95e5d83 but found 4.16.1.cae20318d4.)" but this isn't causing any issues with the editor.

tl;dr - must I update the Wysiwyg module or not in order for my D7 sites to be protected from the issue in SA-CORE-2021-003?

Michael-IDA’s picture

Issue summary: View changes

Hi James,

In #23 did you apply the patch from #10?

I looked at the patch, and yeah I can’t see that it matters at all (other than its complaining messages stop), but I don’t know the Wysiwyg module enough to know if it has additional checks against ‘verified version range’ somewhere else that might be breaking Wysiwyg. That said, I’d most likely guess the breakage is from the up-rev of CKEditor.

And lets be honest, even though D7 EOL has been (re-?)re-extended to November 28, 2022, I’ve found so many bugs in D7 module releases over the last ~18 months that I’ve basically put a ban on updating anything not directly related to security for D7. Which is the long version of saying (like James, Duro, and others) that no I do not want to be required to upgrade to Wysiwyg-dev to apply a security fix for CKEditor.

So, since there does seem to be at least one fix in the -dev branch that is needed to run CKEditor 4.16.1, can we target the fix for this issue, that satisfies #4:

And a new tagged release would be nice when this is fixed so that D7 users do not depend on dev release for this security fix.

such that the minimum amount of change needed to just up-rev CKEditor to 4.16.1 is patched and rolled for a Wysiwyg 7.x-2.7 release.

Best All,
Michael

PS: I’ve edited the Issue summary to propose the above, if this does not meet the maintainers approval, back it out…

Michael-IDA’s picture

Hi awebmanager,

tl;dr - must I update the Wysiwyg module or not in order for my D7 sites to be protected from the issue in SA-CORE-2021-003?

If you can live with the breakage caused by up-rev-ing CKEditor to 4.16.1, then no, you don’t need to update the Wysiwyg module. As James has pointed out, there is some breakage, but it might not effect you...

jamesoakley’s picture

Michael, sorry, my post #23 was less clear than it needed to be. I've updated it to clarify.

I installed the dev release of wysiwyg, and applied the patch from #5, to rule out any possibility that the problem I encountered would be fixed if I used a version incorporating other bug fixes. This was the check that @mcdruid asked me to carry out, but I did the check on a vanilla install of Drupal 7 to be sure no other settings / modules / themes in my site were causing it not to work.

twod’s picture

Wysiwyg module itself is not vulnerable to SA-CORE-2021-003, only the CKEditor library is (in some configurations).

Wysiwyg module does not strictly enforce that you use a library version within the ranges specified in the editor definition to allow you to update them within reasonable limits (usually within major versions).

The version range technically only indicates that a specific version has gone through my suite of automated tests (something I've not had time to do lately) without failing and that I've verified any required changes in plugins/buttons/API/etc have been compensated for, to the best of our abilities (think of it like Drupal's hook_update_N but between library versions - even in reverse).

If you install an editor library outside the version range - given it was still possible to parse the version number - you will get a warning on the profile config page but it will still try to load the editor same as before. This should be safe to do for libraries which follow semantic versioning and don't break backwards compatibility without the corresponding major/minor version number changes.

As far as I can tell there are no API changes or plugin/button definition changes that we should have to compensate for between 4.12.1 and 4.16.1 so installing the latest version "should just work". If you get an error saying it's unable to determine the version number at all it either means the library was unpacked incorrectly or that the version string has moved outside the area of the file being scanned. In the latter case it should be easy to adjust the parsing accordingly. I have some time to investigate this later tonight, but judging by #24 the latest version is already parsed correctly and should thus be working.

If the editor loads after swapping in the new library version and clearing the cache, all should be fine.

jamesoakley’s picture

Status: Needs work » Reviewed & tested by the community

Right, I've found my problem, and others could be caught by the same thing.

It was nothing to do with the module. For some reason, the 4.16.1 version of ckeditor unzips to give directories 700 permission, whereas in the old version all directories had 755. That stopped the webserver from reading the .js files within the ckeditor library. find . -type d -exec chmod 755 {} \; was all I needed.

I've set this back to RTBC

aj2’s picture

Thanks @JamesOakley The directory permissions was also the solution to my problem.

nloomis’s picture

@JamesOakley Permissions are EXACTLY what was wrong!! It was all 700 when the old libraries folders were set to 755. Thanks for posting that!

mcdruid’s picture

Thank you everyone for pitching in.

Can we just get a really clear confirmation on the status?

Is it possible to update to CKEditor 4.16.1 without any changes to the current stable release of this module (7.x-2.6 at present)?

(Noting that @JamesOakley's steps from 29 are likely necessary to fix permissions when a new copy of the library code is put in place).

If not, what other steps do sites need to follow?

jamesoakley’s picture

Is it possible to update to CKEditor 4.16.1 without any changes to the current stable release of this module (7.x-2.6 at present)?

Yes you can.

However, the Wysiwyg Profiles will warn you that they are using a different version of the library from the one they were created with. Ordinarily, the way to remove that warning is to edit the profile, and then save it without any changes. That save operation updates the version of the CKEditor library associated with that profile. That's what you ordinarily do to make the warning go away. With 7.x-2.6 as it stands, 4.16.1 is outside the range of supported library versions, therefore the module will prevent you from saving.

So you can't make that warning go away. But the warning doesn't stop the module from using the library correctly.

twod’s picture

I just noticed that if you download CKEditor from the gihtub releases page that version is in a more "raw" state compared to downloading the FULL package from https://ckeditor.com/ckeditor-4/download/.

The github releases use placeholders instead of the actual version numbers in ckeditor.js so Wysiwyg will not recognize that variant of the library. Even if you manually put the correct version in there instead of the placeholders it won't be loadable by Wysiwyg.
The plugins which are normally bundled inside ckeditor.js are now separate modules and many files would have to be explicitly loaded for it all to work.

Anway, I've checked out 4.16.1 and it does work well with Wysiwyg. There are no relevant API changes since the previously highest verified version number in either the last stable or -dev releases. A new Placeholder plugin was introduced but supporting that feels out of scope for this issue, perhaps later.

Conclusion: Yes you can already use v4.16.1 without modifying this module if you follow these steps:

  • Download the FULL CKEditor 4.16.1 package from https://ckeditor.com/ckeditor-4/download/ (
    Direct link)
  • Unpack/rename/place it in your libraries folder according to the installation instructions on /admin/config/content/wysiwyg.
  • Make sure file/folder permissions allow your webserver to read and serve the files.
  • Verify /admin/config/content/wysiwyg displays CKEditor 4.16.1. You will see a warning but it is harmless.

It got a bit later than I hoped tonight and before formally accepting the patch posted earlier I would like to get my old dev environment fully up and running and do some more checks on the in-between versions as well, but I think I'm too tired for that right now.

Thank you all for reporting, testing, patching, debugging and commenting! See you tomorrow.

devad’s picture

StatusFileSize
new71.22 KB

I have a problem with some button background images not been loaded after I have followed all provided steps described in #34 and updated the library to 4.16.1.

Approximately one half of icons are present and the other half is not. They all work though... it's just background image loading which is problematic. I have tried multiple cache cleans and browser refresh and history clean... but the problem didn't go away. Disabling Drupal core JS compression didn't help as well.

Screenshot attached.

ckeditor

JS console is complaining about exportpdf-no-token-url. but i don't think it is related to background images loading. Here is the error just in case it matters:

ckeditor.js?qtt68e:21 [CKEDITOR] Error code: exportpdf-no-token-url.
(anonymous) @ ckeditor.js?qtt68e:21
ckeditor.js?qtt68e:21 [CKEDITOR] For more information about this error go to https://ckeditor.com/docs/ckeditor4/latest/guide/dev_errors.html#exportpdf-no-token-url
(anonymous) @ ckeditor.js?qtt68e:21

All folders' permissions are 755. If I revert library to my previous ckeditor version (CKEditor 4.4.1.568b5ed) all background images are loaded nicely. Maybe my previous library version is too old for smooth upgrade?

My config: Wysiwyg 7.x-2.6, CKEditor library 4.16.1 (link), Drupal Core 7.80, PHP7.2

twod’s picture

@devad, Thanks for detailing what you've tried.
Something similar usually happen when one accidentally downloads the Basic or Standard version instead of the Full one (I've done it more times than I'd like to admit hehe). You linked to the correct one but can you please double-check?
Something strange is going on with the "Redo" button, you appear to have two of them. Do they both say "Redo" when you hover them?

Can you identify which buttons are missing? If you look in the ckeditor folder there should be a build-config.js which includes a list of all the plugins enabled in the package you got. Can you please post that section of the file here?

Can you screenshot the list of enabled buttons/plugins?

Yes, that message you're seeing is unrelated. It belongs to a plugin which appers to be enabled by default that we could probably disable.

afsch’s picture

StatusFileSize
new630 bytes

Updated patch #5 for wysiwyg 7.x-2.6. There was an error in the path of file in the first line. This allows to update to Wysiwyg 4.16.1.

  • TwoD committed 6513bda on 6.x-2.x authored by 44sunsets
    - #3191083 by 44sunsets, lily.yan, joey-santiago, alexis_saransig, et al...
  • TwoD committed a8d3777 on 7.x-2.x authored by 44sunsets
    - #3191083 by 44sunsets, lily.yan, joey-santiago, alexis_saransig, et al...
twod’s picture

Status: Reviewed & tested by the community » Fixed

I committed #5 since it was the first patch with 4.16.1 which applied cleanly to the 7.x-2.x branch.

I'll look through the issue queu and general status of the module to see if there are any minor quick cleanups needed and then make another release.

Thanks again all for helping out!

(@alexis_saransig, #37 has a wysiwyg path prefix so it does not apply with git, the patch needs to be made from within the module repo.)

devad’s picture

Re #37: @TwoD your post inspired me to do everything again... downloading from your link, unpacking localy, changing permissions, packing again, uploading to server and unpacking on server. At the end everything works fine. All icons are visible. Thank you!

twod’s picture

@devad, great! I forgot to tell you to create a separate issue if the problems persisted, good to know that won't be necessary. :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.