Dependency on phpseclib/phpseclib:dev-master?

[gravelld]

Hi, thanks for this project. Due to these security advisories:

• GHSA-hm7p-r324-hhf3
• GHSA-jpr7-q523-hx25

... we can't use v2.0.4 because it has a dependency on "phpseclib/phpseclib": ">=3.0.13 <4.0.0" (it needs to be "phpseclib/phpseclib": ">=3.0.34 <4.0.0".

However, this project's master has a dependency on phpseclib/phpseclib:dev-master. And phpseclib/phpseclib's README.MD says:

master

Development Branch
Unstable API
Do not use in production

... so I can't use that either. Is there a recommendation what to do in this situation?

[terrafrost]

... we can't use v2.0.4 because it has a dependency on "phpseclib/phpseclib": ">=3.0.13 <4.0.0" (it needs to be "phpseclib/phpseclib": ">=3.0.34 <4.0.0".

3.0.34 is >=3.0.13 and <4.0.0 so I'm not seeing a problem

[gravelld]

The dependency requirement is:

"phpseclib/phpseclib": ">=3.0.13 <4.0.0"

This therefore allows versions between 3.0.13 and 3.0.34 exclusive. Static analysis tools read this requirement and flag it as a vulnerability because there's a chance a vulnerable version (prior to 3.0.34) of phpseclib/phpseclib will be chosen.

This is the case with release 2.0.4 of mcrypt_compat. If a new version was made available with this requirement:

"phpseclib/phpseclib": ">=3.0.34 <4.0.0"

It would solve the problem (although note there's no 4.0.0...). Perhaps the requirement should be:

"phpseclib/phpseclib": "^3.0.34"

(ref: version constraint syntax)

?

[terrafrost]

Static analysis tools read this requirement and flag it as a vulnerability because there's a chance a vulnerable version (prior to 3.0.34) of phpseclib/phpseclib will be chosen.

imho that's beyond the purview of static analysis tools. If you want to make sure you're not running a vulnerable version of a library install https://github.com/Roave/SecurityAdvisories . That way you're not being red flagged because you could be running a vulnerable version of a library - you're being red flagged if you are running a vulnerable version of a library.

There's a chance you could be using a version of PHP with a known vulnerability as well. Certainly there are some that think that all versions of phpseclib should drop support for PHP < 8.1.27, PHP >= 8.2.0 < 8.2.14 and PHP >= 8.3.0 < 8.3.1 (ie. any non current version of PHP) but in the interests of backwards compatibility I'm not going to be doing that.

Plus, I feel like most anyone using mcrypt_compat, in particular, is liable to be red flagged for using "mcrypt", at all. There are no shortage of people on wordpress.org making support requests talking about how some plugin that they're using is being red flagged because that plugin, vis-a-vis phpseclib, is using mcrypt, even tho phpseclib only calls the mcrypt functions if function_exists('mcrypt_encrypt') or whatever.

I mean, I'm not opposed to making this change, per say. If lots of people start making the same request I'd be more willing to do it. But I'm also not a big fan of making a change just for the sake of making a change, which is what this feels like.

That said, I did do some testing. So let's say you're running phpseclib 3.0.20. If you then do composer require phpseclib/mcrypt2_compat:~2.0 the version of phpseclib that you're using won't be updated. With your proposal it wouldn't fail - it'd find the most recent version that it could install, in this case, 2.0.4. So there's that, at least. And the reason I like that behavior is that I wouldn't want to force anyone to do a composer update because, best intentions notwithstanding, libraries still do sometimes break semver and doing gratuitous composer updates can break stuff. And sure, if that's the case, you ought to either (1) update your composer.json to use a specific version or (2) you should update your code to work with the newest version but in the case of (1) you'd have to figure out what that right version is and in the case of (2) that's dev time that you may or may not have, as a matter of practicality.

But I'm still leaning towards not doing it, currently, unless more people ask me too, because I'm not so sure that static analyzers should be making that kind of check (https://github.com/Roave/SecurityAdvisories is the better solution based on my current line of thinking) and I don't necessarily want to encourage behavior that I don't completely agree with lol.

I'll mull on it some more over the holiday weekend.

[gravelld]

Thanks for your time to consider this. I'm forced to use this tool by the body that want to audit our code, so it's not a solution "choice" we have. See https://appdefensealliance.dev/casa/tier-2/ast-guide if you're interested.

I totally understand your balancing of library maintenance and change-for-changes sake.

[terrafrost]

2.0.5 bumps the minimum required version of phpseclib/phpseclib.