[wp-trac] [WordPress Trac] #48955: WP 5.3.1 changes cause potential backwards compatibility breakage with kses

[WordPress Trac]

#48955: WP 5.3.1 changes cause potential backwards compatibility breakage with kses
--------------------------+---------------------
 Reporter:  iCaleb        |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  5.3.3
Component:  Security      |     Version:  5.3.1
 Severity:  normal        |  Resolution:
 Keywords:  needs-patch   |     Focuses:
--------------------------+---------------------

Comment (by aduth):

 What about plugins which respect the documented string type, and would
 likewise break if a feature enhancement to support arrays was adopted?

 For example:

 https://github.com/Automattic/jetpack/blob/e67bea6/modules/shortcodes/youtube.php#L109
 Filtering as string:
 https://github.com/Automattic/jetpack/blob/e67bea6/modules/shortcodes/youtube.php#L37

 https://plugins.trac.wordpress.org/browser/category-
 icons/trunk/category_icons.php?rev=744383#L37
 Filtering as string: https://plugins.trac.wordpress.org/browser/category-
 icons/trunk/category_icons.php?rev=744383#L447

 https://plugins.trac.wordpress.org/browser/twitter-embed/trunk/twitter-
 embed.php?rev=2074143#L29
 Filtering as string: https://plugins.trac.wordpress.org/browser/twitter-
 embed/trunk/twitter-embed.php?rev=2074143#L47

 https://plugins.trac.wordpress.org/browser/article-directory/trunk
 /article-directory.php?rev=333204#L37
 Filtering as string: https://plugins.trac.wordpress.org/browser/article-
 directory/trunk/article-directory.php?rev=333204#L33

 https://plugins.trac.wordpress.org/browser/yubikey-
 plugin/trunk/yubikey.php?rev=832195#L421
 Filtering as string: https://plugins.trac.wordpress.org/browser/yubikey-
 plugin/trunk/yubikey.php?rev=832195#L407

 https://plugins.trac.wordpress.org/browser/superpack/tags/0.3.1/inc/class-
 superpack-shortcodes.php#L67
 Filtering as string:
 https://plugins.trac.wordpress.org/browser/superpack/tags/0.3.1/inc/class-
 superpack-shortcodes.php#L101

 https://plugins.trac.wordpress.org/browser/another-soundcloud-
 quicktag/trunk/another-soundcloud-quicktag.php?rev=333652#L30
 Filtering as string: https://plugins.trac.wordpress.org/browser/another-
 soundcloud-quicktag/trunk/another-soundcloud-quicktag.php?rev=333652#L33

 https://plugins.trac.wordpress.org/browser/shortcake-bakery/trunk/inc
 /class-shortcake-bakery.php?rev=1649994#L77
 Filtering as string: https://plugins.trac.wordpress.org/browser/shortcake-
 bakery/trunk/inc/class-shortcake-bakery.php?rev=1649994#L105

 https://plugins.trac.wordpress.org/browser/article-directory-redux/trunk
 /icwp-adr.php?rev=900910#L67
 Filtering as string: https://plugins.trac.wordpress.org/browser/article-
 directory-redux/trunk/icwp-adr.php?rev=900910#L189

 https://plugins.trac.wordpress.org/browser/shoplocket/trunk/shoplocket.php?rev=688408#L44
 Filtering as string:
 https://plugins.trac.wordpress.org/browser/shoplocket/trunk/shoplocket.php?rev=688408#L187

 https://plugins.trac.wordpress.org/browser/wpdirectory/trunk/wpdirectory.php?rev=185104#L29
 Filtering as string:
 https://plugins.trac.wordpress.org/browser/wpdirectory/trunk/wpdirectory.php?rev=185104#L25

 Arguably, it was never the case that `wp_kses` would have worked as
 expected with an array value filtered by any of the plugins above.

 The reason I keep harping on about the documentation is that it serves as
 a contract against which backwards-compatibility should be measured. It
 acts as a promise that the framework will behave in a particular way under
 a specific set of conditions, of which the given arguments shape is one
 such condition. For plugins which passed an array value, there was no
 guarantee on the behavior, in much the same way that I could call
 `wp_insert_post( new DateTime() )`, and it might do something different
 years from now than it does today.

 If a plugin comes to rely on an incidental, undocumented behavior, it's
 unfortunate if that behavior changes over time. But for each one of those
 cases, there are many more where a plugin does target the documented
 signature, and may similarly break if we were to change the behavior of
 the function. Given the choice, we should seek to uphold support for the
 documented behavior.

 There's also a question of just how far feature support should extend.
 Without delving into specifics, I can already imagine at least one
 potential security concern that could result from the proposed enhancement
 to KSES functions. This leads me to resurface a
 [https://wordpress.slack.com/archives/C02RQBWTW/p1576528487144300 point I
 had raised in a recent Slack conversation]
 ([https://make.wordpress.org/chat/ link requires registration]), wherein
 we should be especially cautious with added feature support for these
 functions, given their sensitive nature and the expanded complexity of
 supporting additional combinations of arguments.

 If we treat this as a feature enhancement, it should be evaluated on its
 own merits, and implemented in mind of the potential impact on existing
 usage as documented. On that basis, I'm personally not convinced it should
 be supported.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/48955#comment:21>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform