There is an XSS vulnerability in the league/commonmark library in versions < 0.18.1: https://github.com/thephpleague/commonmark/issues/337.

It has been fixed in version 0.18.1. However, for markdown version 8.x-1.x the required version of this library is ^0.15.0. We need to update to the latest version and since it is a jump from 0.15 to 0.18, we need to rigorously test it to make sure it does not have any breaking changes which could make the module unusable.

CommentFileSizeAuthor
#15 3024662-15.patch322 byteskim.pepper
#6 markdown-3024662-6.patch322 bytesptmkenny
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

shabana.navas created an issue. See original summary.

shabana.navas’s picture

Issue summary: View changes
markhalliwell’s picture

Category: Bug report » Support request
Priority: Critical » Minor
Status: Active » Closed (works as designed)

Version management of external libraries is handled by composer. You can already install a higher version if you need to (in this case for sec reasons).

I follow that project fairly closely and Colin has already promised that the 0.x versions will remain BC from 0.15 onward.

I already have plans to update the "install" version to the latest release in #2952435: Merge in the CommonMark project (which I'm still working on, albeit... not my biggest priority ATM).

markhalliwell’s picture

Title: XSS Vulnerability in Commonmark » Update league/commonmark library to >= 0.18.1
Version: 8.x-1.x-dev » 8.x-2.x-dev
Category: Support request » Task
Priority: Minor » Normal
Status: Closed (works as designed) » Active

Regardless, this still needs to happen. Best to not forget about it, just in case.

ptmkenny’s picture

I tried to update commonmark with composer, but when I run composer update league/commonmark, I get "nothing to update."

So I tried to install it manually like this:

:/mnt/c/d/mysite$ composer require league/commonmark
Using version ^0.18.1 for league/commonmark
./composer.json has been updated
Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies (including require-dev)
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - drupal/markdown 1.2.0 requires league/commonmark ^0.15.0 -> satisfiable by league/commonmark[0.15.0, 0.15.1, 0.15.2, 0.15.3, 0.15.4, 0.15.5, 0.15.6, 0.15.7] but these conflict with your requirements or minimum-stability.
    - drupal/markdown 1.2.0 requires league/commonmark ^0.15.0 -> satisfiable by league/commonmark[0.15.0, 0.15.1, 0.15.2, 0.15.3, 0.15.4, 0.15.5, 0.15.6, 0.15.7] but these conflict with your requirements or minimum-stability.
    - drupal/markdown 1.2.0 requires league/commonmark ^0.15.0 -> satisfiable by league/commonmark[0.15.0, 0.15.1, 0.15.2, 0.15.3, 0.15.4, 0.15.5, 0.15.6, 0.15.7] but these conflict with your requirements or minimum-stability.
    - Installation request for drupal/markdown ^1.2 -> satisfiable by drupal/markdown[1.2.0].


Installation failed, reverting ./composer.json to its original content.

This particular example is from 1.2, but I get a similar error with 2.0-alpha1.

ptmkenny’s picture

Status: Active » Needs review
FileSize
322 bytes

Attaching a patch for 1.x-dev. Unfortunately, this doesn't work as a patch until the module has a release, because by the time the module is patched, composer has already calculated the dependencies.

kim.pepper’s picture

Priority: Normal » Critical

@markcarver As per #5 It's still not possible to install this with a secure version of league/commonmark

Bumping to critical because of this.

kim.pepper’s picture

Status: Needs review » Reviewed & tested by the community
geek-merlin’s picture

To sort this out: it looks to me that this is fixed:
* composer.json · 8.x-2.0-alpha1 · project / markdown · GitLab has "league/commonmark": "^0.17.1|^1.0"
* composer.json · 8.x-2.x · project / markdown · GitLab has "league/commonmark": ">=0.18.0"

whyever... what do you say?

kim.pepper’s picture

According to https://github.com/thephpleague/commonmark/issues/337 this is fixed in 0.18.1. The current recommended version of 2.0-alpha1 has a version constraint "league/commonmark": "^0.17.1|^1.0" which does not allow you to install ^0.18.1.

I get the following error when using the 2.x dev version

Fatal error: Declaration of Drupal\markdown\Plugin\Markdown\Extension\AtAutolinker::getCharacters() must be compatible with League\CommonMark\Inline\Parser\InlineParserInterface::getCharacters(): array in /data/app/modules/contrib/markdown/src/Plugin/Markdown/Extension/AtAutolinker.php on line 23

mxr576’s picture

Can we get this fix merged?

geek-merlin’s picture

> The current recommended version of 2.0-alpha1 has a version constraint "league/commonmark": "^0.17.1|^1.0" which does not allow you to install ^0.18.1.

Ah OK, get it: According to the composer docs ^0.17.1 means <0.18...

> Can we get this fix merged?

The fix is in dev, so we should issue-request a stable release...

> I get the following error when using the 2.x dev version

...after that bug is fixed.

Please open an issue for it!

mxr576’s picture

malcolm_p’s picture

It sounds like the 1.x version needs to be updated to ~0.15.0 < 0.19.0 based on the 0.19 breaking change.

kim.pepper’s picture

Version: 8.x-2.x-dev » 8.x-1.x-dev
FileSize
322 bytes

This was originally posted as a 8.x-1.x issue and it still applies to that branch, so switching it back.

markhalliwell’s picture

Title: Update league/commonmark library to >= 0.18.1 » Update league/commonmark library to ^0.18.3

  • markcarver committed ecc3916 on 8.x-1.x authored by kim.pepper
    Issue #3024662 by kim.pepper, ptmkenny: Update league/commonmark library...
markhalliwell’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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