Skip to content

Commit

Permalink
ci: Drop PHP 8.1 from our tests
Browse files Browse the repository at this point in the history
Under Ubuntu 24.04 the pytest for
test/test_php_isolation.py::test_php_isolation_rootfs fails due to Unit
aborting (SIGABRT) in the PHP language module due to FORIFY_SOURCE
hardening detecting a buffer overflow

  2024/10/16 16:46:54 [info] 11661#11661 "phpinfo" application started
  *** buffer overflow detected ***: terminated
  2024/10/16 16:46:54 [alert] 11660#11660 app process 11661 exited on signal 6

After spending an extraordinary amount of time faffing around with
Ubuntu and pytests (they don't make for a pleasant combination) I was
able to reproduce it.

The crash was occurring here

  #4  0x00007ebe818288ff in __GI_abort () at ./stdlib/abort.c:79
  #5  0x00007ebe818297b6 in __libc_message_impl (
      fmt=fmt@entry=0x7ebe819ce765 "*** %s ***: terminated\n")
      at ../sysdeps/posix/libc_fatal.c:132
  #6  0x00007ebe81936c19 in __GI___fortify_fail (
      msg=msg@entry=0x7ebe819ce74c "buffer overflow detected")
      at ./debug/fortify_fail.c:24
  #7  0x00007ebe819365d4 in __GI___chk_fail () at ./debug/chk_fail.c:28
  #8  0x00007ebe8134a055 in mempcpy (__len=10, __src=0x7ebe8160ade8,
      __dest=0x571ba9bd0930)
      at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:45
  #9  fake_data_segment (info=0x0, sysdb=0x571ba9bcf080)
      at /usr/src/php8.1-8.1.30-1+ubuntu24.04.1+deb.sury.org+1/ext/date/lib/parse_tz.c:921
  #10 timelib_builtin_db ()
      at /usr/src/php8.1-8.1.30-1+ubuntu24.04.1+deb.sury.org+1/ext/date/lib/parse_tz.c:1084
  #11 0x00007ebe812e0885 in zm_info_date (zend_module=0x571ba9a14420)

[Well as best as I can tell, as this is from the php 8.1 packages from
<https://github.com/oerdnj/deb.sury.org>, I don't know where the
packages (I'm assuming it's packages) shivammathur/setup-php@v2
installs come from.]

So we get killed in fake_data_segment(), the thing is, that function (as
well as timelib_builtin_db()) doesn't exist in upstream PHP.

It turns out these come from a patch that is applied by distributions to
make PHP use the system installed timezone database rather than the one
built into PHP.

I was unable to reproduce this with vanilla PHP 8.1.

It can be triggered on affected builds with the following config

  {
      "listeners": {
          "[::1]:8080": {
              "pass": "applications/php"
          }
      },

      "applications": {
          "php": {
              "type": "php",
              "root": "/app/php",

      	      "isolation": {
  	          "rootfs": "/tmp/unit-root",

                  "namespaces": {
  	              "mount": true,
  		      "credential": true,
  		      "pid": true
                  }
  	      }
          }
      }
  }

The crux of the issue seems to come down to in this case PHP can't open
the tz database as it's not contained in the new mount namespace.

  190437 openat(AT_FDCWD, "/usr/share/zoneinfo/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory)
  190437 openat(AT_FDCWD, "/usr/share/zoneinfo/zone.tab", O_RDONLY) = -1 ENOENT (No such file or directory)
  190437 writev(2, [{iov_base="*** ", iov_len=4}, {iov_base="buffer overflow detected", iov_len=24}, {iov_base=" ***: terminated\n", iov_len=17}], 3) = 45
  ...
  190437 --- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=2, si_uid=65534} ---
  190437 +++ killed by SIGABRT +++

Specifically the issue is with the following code in the patch
(certainly an earlier version of the patch, this is from a Debian patch
<https://sources.debian.org/src/php8.2/8.2.20-1~deb12u1/debian/patches/0007-Add-support-for-use-of-the-system-timezone-database.patch/>)

  +        data = malloc(3 * sysdb->index_size + 7);
  +
  +        p = mempcpy(data, FAKE_HEADER, sizeof(FAKE_HEADER) - 1);

If the zone file hasn't been found then sysdb->index_size is 0. So we
malloc(3) a total of 7 bytes.

However, sizeof(FAKE_HEADER) - 1 is 10. (Hence the __len=10 in the
mempcpy(3) in the above backtrace).

Of course 10 doesn't fit into 7 and the FORTIFY_SOURCE hardening kicks
in and SIGABRTs the process.

Now, it's worth noting that this issue doesn't occur with PHP 8.2 and
8.3.

As can been seen from the Fedora patch for this
<https://src.fedoraproject.org/rpms/php/blob/rawhide/f/php-8.4.0-systzdata-v24.patch>

They actually have a fix incorporated

  r23: fix possible buffer overflow

So the above patch now does

  +        data = malloc(3 * sysdb->index_size + sizeof(FAKE_HEADER) - 1);
  +
  +        p = mempcpy(data, FAKE_HEADER, sizeof(FAKE_HEADER) - 1);

So you will always get at least the required 10 bytes allocated.

I assume the PHP 8.2 & 8.3 packages either no longer use this patch or
have the fixed version. I don't know... I haven't found the sources...

Anyway the above was more about satisfying myself that the problem
wasn't with Unit.

PHP 8.1 is now in security maintenance mode and people are actively
encouraged to upgrade to 8.2/8.3

So lets just remove 8.1 from our testing...

[It's also worth noting that after all this, the ubuntu-latest runners
seemed to have switched back from 24.04 to 22.04. However lets stick
with this and the other ci fixes as who knows when it'll go back to
24.04 (or some other version) again...]

Link: <https://www.php.net/supported-versions.php>
Signed-off-by: Andrew Clayton <[email protected]>
  • Loading branch information
ac000 committed Oct 16, 2024
1 parent 4601db6 commit 75b7231
Showing 1 changed file with 0 additions and 2 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ jobs:
os: ubuntu-latest
- build: perl
os: ubuntu-latest
- build: php-8.1
os: ubuntu-latest
- build: php-8.2
os: ubuntu-latest
- build: php-8.3
Expand Down

0 comments on commit 75b7231

Please sign in to comment.