search

wp-cli / i18n-command

Provides internationalization tools for WordPress projects.
MIT License
96 stars 52 forks source link

Use `preg_match` instead of `mb_ereg` #317

Closed swissspidy closed 2 years ago

swissspidy commented 2 years ago

Fixes #117

schlessera commented 2 years ago

This just gets rid of multibyte support for the regular expressions. Is this the right fix here? I guess it is unlikely to have issues from that due to the fact we're string-matching a full filename or similar against a filepath - but it is not entirely impossible.

Is it technically possible this introduces security issues with malicious filesystem traversal because of bad truncating, for example?

@swissspidy Thoughts?

swissspidy commented 2 years ago

I guess the only other viable solution is to use mb_ereg everywhere and not use PCRE at all, so that usage is consistent and not mixed. But then we should probably make it a required extension.

Not too familiar with either to be able to judge the security implications tbh.

schlessera commented 2 years ago

Going all-in with mb_ereg() still doesn't solve the issue with preg_quote(), as there is not alternative for mb_* functions.

Merging this for now, but it will probably need to be testing in international setups to see if everything still works as expected.