php / pie

The PHP Installer for Extensions
BSD 3-Clause "New" or "Revised" License
666 stars 19 forks source link

Don't download Windows packages from GH release assets #86

Closed cmb69 closed 1 week ago

cmb69 commented 2 weeks ago

https://github.com/ThePHPF/pie-design?tab=readme-ov-file#windows-binaries mentions that Windows packages should be downloaded from GH release assets. In my opinion, that is a bad idea for (a) security and (b) stability reasons.

Re (a): a malicious actor could register a useful extension via packagist, and then roll out Windows binaries as releases which may contain arbitrary malware (those binaries would not even need to be built from the GH repository sources). There is no way to verify the integrity of the binaries.

Re (b): even if we assume no malicious intententions, Windows binaries may be built against arbitrary dependendencies, and these may conflict with others. E.g. two different extensions might use the same dependency DLL and ship it; but the first one uses a newer version, and the second one an old version. The first one might rely on functions of the newer version, but if the second extension is installed after the first one, the DLL will be replaced, and the first extension will no longer work. Even if we assume that this won't happen, there may be subtle differences regarding the used Windows SDK etc.

I suggest to only download Windows binaries from PECL (i.e. from https://downloads.php.net/~windows/pecl/releases/). These are under the control of the PHP organization (particularly how they are built), and could be easily rebuilt in case of any conflicts (or withdrawn if need be).

cc @shivammathur

asgrim commented 1 week ago

Firstly, thank you for your feedback, perhaps it could've come a little sooner though :) ... the design of PIE was discussed almost a year ago with public consultation, and the https://github.com/ThePHPF/pie-design repo has been worked on transparently and in public, with ample opportunity for much earlier feedback.

Both points (a) and (b), as far as I can tell, could equally apply to downloading from https://downloads.php.net/~windows/pecl/releases/ .

For example (a) less people have access, but that does not mean a bad actor could take advantage of a hypothetical vulnerability and place their own malicious binaries. This point also applies to any extension built from source, in practice, since (unfortunately) very few people using PECL (or PIE, eventually) will actually read the source before compiling anything.

(b) also applies because the extension binary is built against whatever libraries builds them... which may be different for each extension there. This issue doesn't seem any different when building for the official Windows binaries provided, versus any other way of providing binaries.

The recommended approach in PIE for building the Windows binaries is to use https://github.com/php/php-windows-builder ; which means the PHP organisation is already in control (as much as extension authors can opt in to using it) the stack on which extensions are built; but this gives extension authors the freedom to produce their own binaries, should they wish.

That said, there is scope in future for improvements, such as;

I will move this into the PIE design repo. oops, looks like I can't transfer across organisations (makes sense). This is really a design discussion, rather than a bug. I will close this and open a new issue in the correct repo

asgrim commented 1 week ago

Further discussion should be on: ThePHPF/pie-design#27