php: Fix CVE-2018-17082 (nixos-unstable) #50511

Ekleog · 2018-11-17T15:44:40Z

php: 7.2.10 -> 7.2.12, 7.1.22 -> 7.1.24

Also make Darwin align itself to Linux versions, due to CVE-2018-17082
forcing our hand on this. This means Darwin must compile without intl.

Motivation for this change

The alternative would be to backport the fix to a previous version of php. It looks pretty easy, but I know the PHP code base very little and wouldn't trust simply using this patch to fix the issue unless spending much more time on it.

cc @etu who introduced the split between darwin and linux

@GrahamcOfBorg build php71 php72
@GrahamcOfBorg test elk owncloud

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Determined the impact on package closure size (by running nix path-info -S before and after)
Fits CONTRIBUTING.md.

Ekleog · 2018-11-17T15:51:25Z

See also backports at #50505 and (almost-a-backport) #50377.

GrahamcOfBorg · 2018-11-17T16:07:25Z

Success on aarch64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12/bin/pear: interpreter directive changed from "/bin/sh" to "/nix/store/dsyc1z7ck08ga7l0b1jcxx35wj69qcii-bash-4.4-p23/bin/sh"
/nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12/bin/php-config: interpreter directive changed from " /bin/sh" to "/nix/store/dsyc1z7ck08ga7l0b1jcxx35wj69qcii-bash-4.4-p23/bin/sh"
/nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12/bin/phpize: interpreter directive changed from "/bin/sh" to "/nix/store/dsyc1z7ck08ga7l0b1jcxx35wj69qcii-bash-4.4-p23/bin/sh"
checking for references to /build in /nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12...
moving /nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12/sbin/* to /nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12/bin
shrinking RPATHs of ELF executables and libraries in /nix/store/83zhn2cjrglp1s6dsq00da8mz2qa5xj5-php-7.2.12-dev
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/83zhn2cjrglp1s6dsq00da8mz2qa5xj5-php-7.2.12-dev
checking for references to /build in /nix/store/83zhn2cjrglp1s6dsq00da8mz2qa5xj5-php-7.2.12-dev...
/nix/store/fwsncrrpccpjd1dq4jqqkwrz1b687hb8-php-7.2.12

GrahamcOfBorg · 2018-11-17T16:29:56Z

Success on x86_64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12/bin/php-config: interpreter directive changed from " /bin/sh" to "/nix/store/r47p5pzx52m3n34vdgqpk5rvqgm0m24m-bash-4.4-p23/bin/sh"
/nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12/bin/peardev: interpreter directive changed from "/bin/sh" to "/nix/store/r47p5pzx52m3n34vdgqpk5rvqgm0m24m-bash-4.4-p23/bin/sh"
/nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12/bin/pecl: interpreter directive changed from "/bin/sh" to "/nix/store/r47p5pzx52m3n34vdgqpk5rvqgm0m24m-bash-4.4-p23/bin/sh"
checking for references to /build in /nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12...
moving /nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12/sbin/* to /nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12/bin
shrinking RPATHs of ELF executables and libraries in /nix/store/38xykx7qnndkmfkwq0sgndmsdmh5yy19-php-7.2.12-dev
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/38xykx7qnndkmfkwq0sgndmsdmh5yy19-php-7.2.12-dev
checking for references to /build in /nix/store/38xykx7qnndkmfkwq0sgndmsdmh5yy19-php-7.2.12-dev...
/nix/store/z38071vwz4kvhqiyv3qf89nqkjz185l5-php-7.2.12

etu · 2018-11-17T20:13:37Z

@Ekleog I first used a non-assert solution and got it pointed out to me that that was a bad idea because then it goes unnoted that the change of feature flags have happened. Which I agree to.

That's why it ended up like this but got documented. And I've been following the issue on the PHP bugtracker to see if they actually will resolve this but it's not very hopeful. Which actually kinda surprise me since they broke the build in a minor release and don't seem able to fix it.

I'm also in favor of aligning it up again and if you and others think that this assert warning solution is good enough for this, I'm all for it.

I've also looked through the intl extension in the docs a bit https://secure.php.net/intl and, I must say. From my years (8+) of working professionally with PHP. I don't think that I've ever to my knowledge have used this extension myself. It may be used by some popular libraries, I don't know about that.

But overall I'm in favor of this.

With this fix we could close #50014.

Ekleog · 2018-11-18T16:00:11Z

@etu Hmm, it's possible to add the assert and require that intl is manually set to false… would that do the job? It may be a harsh bump for stable-channel users, but it's likely better than leaving it go unnoticed indeed? Also, Darwin users are more or less pushed towards nixpkgs-unstable anyway IIRC, so it may be less big a deal than it'd have been had it been on all linuxes.

etu · 2018-11-18T19:12:48Z

@Ekleog As the person who pointed me out to me said: "Better to fail on rebuild than the user upgrading and go home for the weekend and see it fail while it's weekend."

etu · 2018-11-21T13:47:17Z

@Ekleog Status on this?

Also make Darwin align itself to Linux versions, due to CVE-2018-17082 forcing our hand on this. This means Darwin must compile without intl.

Ekleog · 2018-11-25T13:51:13Z

@etu Sorry for the delay, I was away from the Internet last week! I think unstable should go without the manual change, because the default will need to change before 19.03 anyway, and unstable users should be aware of this kind of things.

So I've pushed two new versions of this PR, for unstable with the change of default value and an error if intl is forced to true, and for 18.09 with no change of the default value and an error if intl is not forced to false.

This will likely break tests for 18.09, unfortunately… but I don't really see a better way forward, as silent breakage or leaving a security vulnerability are two important issues… I guess we'll have to fix tests once we notice they break, though I'm not sure how often tests are run for Darwin on 18.09.

TL;DR: I think this is good to go, and we'll have to handle the broken stuff when it becomes apparent.

Also make Darwin align itself to Linux versions, due to CVE-2018-17082 forcing our hand on this. This means Darwin must compile without intl. Based on commit 283396658a69f5a4ed3d832bc849c53fa8e05ce1, with the change that here we refuse to build if the user doesn't manually disable intl. See [1] for the reasoning. [1] NixOS#50511 (comment)

GrahamcOfBorg · 2018-11-25T14:10:07Z

Success on x86_64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12/lib/build/config.guess: interpreter directive changed from " /bin/sh" to "/nix/store/n2hjbpkf4c0m48945ivxs3lmsczzw2rg-bash-4.4-p23/bin/sh"
/nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12/lib/build/shtool: interpreter directive changed from "/bin/sh" to "/nix/store/n2hjbpkf4c0m48945ivxs3lmsczzw2rg-bash-4.4-p23/bin/sh"
/nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12/lib/build/config.sub: interpreter directive changed from " /bin/sh" to "/nix/store/n2hjbpkf4c0m48945ivxs3lmsczzw2rg-bash-4.4-p23/bin/sh"
checking for references to /build in /nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12...
moving /nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12/sbin/* to /nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12/bin
shrinking RPATHs of ELF executables and libraries in /nix/store/kr0bbcgkpxgiajg19kpgbx3l4yi1g1nj-php-7.2.12-dev
strip is /nix/store/rpbg8gmqxhz8g61p1plz5d2srs84pvmv-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/kr0bbcgkpxgiajg19kpgbx3l4yi1g1nj-php-7.2.12-dev
checking for references to /build in /nix/store/kr0bbcgkpxgiajg19kpgbx3l4yi1g1nj-php-7.2.12-dev...
/nix/store/bm6yqn88skmdp2a7w9lrc0abawai52js-php-7.2.12

GrahamcOfBorg · 2018-11-25T14:10:30Z

Success on aarch64-linux (full log)

Attempted: php

Partial log (click to expand)

/nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12/bin/pear: interpreter directive changed from "/bin/sh" to "/nix/store/n1kfdl37qpzh3xn6klbym1ay6xpxvmw1-bash-4.4-p23/bin/sh"
/nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12/bin/php-config: interpreter directive changed from " /bin/sh" to "/nix/store/n1kfdl37qpzh3xn6klbym1ay6xpxvmw1-bash-4.4-p23/bin/sh"
/nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12/bin/phpize: interpreter directive changed from "/bin/sh" to "/nix/store/n1kfdl37qpzh3xn6klbym1ay6xpxvmw1-bash-4.4-p23/bin/sh"
checking for references to /build in /nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12...
moving /nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12/sbin/* to /nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12/bin
shrinking RPATHs of ELF executables and libraries in /nix/store/cyi8mnwl5d52nan88dp350lkhg41sfi6-php-7.2.12-dev
strip is /nix/store/6dpnd5aniypn8124mmy8f88s4mq2zl07-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/cyi8mnwl5d52nan88dp350lkhg41sfi6-php-7.2.12-dev
checking for references to /build in /nix/store/cyi8mnwl5d52nan88dp350lkhg41sfi6-php-7.2.12-dev...
/nix/store/hpw3ysi8khgffm75y3v64xyml53avjw7-php-7.2.12

etu · 2018-11-25T16:32:24Z

Yeah, there's those two choices. And neither choice is a good choice.

So one have to pick what's best. And it this case I agree that it's different on unstable and stable 🙂

delroth · 2018-11-27T00:48:10Z

Counterproposal on #51091 to keep backwards compatibility -- the patches are trivial enough that imo it's worth it. I'll let you pick your favorite option :)

Ekleog · 2018-11-27T23:02:07Z

Superseded by now-merged #51091

Ekleog changed the title ~~php: 7.2.10 -> 7.2.12, 7.1.22 -> 7.1.24~~ php: Fix CVE-2018-17082 Nov 17, 2018

Ekleog mentioned this pull request Nov 17, 2018

php: Fix CVE-2018-17082 (release-18.09) #50505

Closed

9 tasks

Ekleog changed the title ~~php: Fix CVE-2018-17082~~ php: Fix CVE-2018-17082 (nixos-unstable) Nov 17, 2018

Ekleog mentioned this pull request Nov 17, 2018

php: Fix CVE-2018-17082 (release-18.03) #50377

Closed

9 tasks

Ekleog mentioned this pull request Nov 17, 2018

Vulnerability roundup 52: php-7.2.8: 1 advisory #50368

Closed

1 task

GrahamcOfBorg added 10.rebuild-darwin: 11-100 10.rebuild-linux: 101-500 labels Nov 17, 2018

Ekleog force-pushed the php-cve-2018-17082-unstable branch 2 times, most recently from 2833966 to 344d88d Compare November 25, 2018 13:43

GrahamcOfBorg added 6.topic: nixos 8.has: changelog 8.has: documentation 2.status: merge conflict labels Nov 25, 2018

Ekleog force-pushed the php-cve-2018-17082-unstable branch from 344d88d to d0bdf4d Compare November 25, 2018 13:50

GrahamcOfBorg added 10.rebuild-linux: 11-100 and removed 10.rebuild-linux: 101-500 2.status: merge conflict labels Nov 25, 2018

etu approved these changes Nov 25, 2018

View reviewed changes

delroth mentioned this pull request Nov 27, 2018

php: 7.2.11 -> 7.2.12, 7.1.23 -> 7.1.24 (CVE-2018-17082) #51091

Merged

10 tasks

Ekleog closed this Nov 27, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

php: Fix CVE-2018-17082 (nixos-unstable) #50511

php: Fix CVE-2018-17082 (nixos-unstable) #50511

Ekleog commented Nov 17, 2018 •

edited

Loading

Ekleog commented Nov 17, 2018

GrahamcOfBorg commented Nov 17, 2018

GrahamcOfBorg commented Nov 17, 2018

etu commented Nov 17, 2018

Ekleog commented Nov 18, 2018

etu commented Nov 18, 2018

etu commented Nov 21, 2018

Ekleog commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

etu commented Nov 25, 2018

delroth commented Nov 27, 2018

Ekleog commented Nov 27, 2018

php: Fix CVE-2018-17082 (nixos-unstable) #50511

php: Fix CVE-2018-17082 (nixos-unstable) #50511

Conversation

Ekleog commented Nov 17, 2018 • edited Loading

php: 7.2.10 -> 7.2.12, 7.1.22 -> 7.1.24

Motivation for this change

Things done

Ekleog commented Nov 17, 2018

GrahamcOfBorg commented Nov 17, 2018

GrahamcOfBorg commented Nov 17, 2018

etu commented Nov 17, 2018

Ekleog commented Nov 18, 2018

etu commented Nov 18, 2018

etu commented Nov 21, 2018

Ekleog commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

GrahamcOfBorg commented Nov 25, 2018

etu commented Nov 25, 2018

delroth commented Nov 27, 2018

Ekleog commented Nov 27, 2018

Ekleog commented Nov 17, 2018 •

edited

Loading