gnutar: disable fortify3 hardening flag

[kirillrdy]

Description of changes

after #224822

tar crashes during building of docker image, sorry I don't have a small test to reproduce (edit: here is small test to reproduce https://discourse.nixos.org/t/cant-get-coredump-from-nixbld-user/31090 , but this fixes the crash for me

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 23.11 Release Notes (or backporting 23.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

[vcunat]

Maybe that's a genuine bug in tar? (their buffer overflowing)

[vcunat]

I mean, the whole point of this hardening is to cause (safe) crashes.

[kirillrdy]

I mean, the whole point of this hardening is to cause (safe) crashes.

fair enough, I will have a closer look at crash.

makes me wonder what it writes to the tar if it doesn't crash !

[kirillrdy]

I have created minimal derivation to reproduce the issue, but i am having difficulty getting a coredump

https://discourse.nixos.org/t/cant-get-coredump-from-nixbld-user/31090

any help appreciated

[puckipedia]

The issue seems to be a bug in fakechroot; its shim of __readlinkat_chk seems to call the next readlinkat with a buffer size of FAKECHROOT_PATH_MAX-1, but with the buflen equivalent to the smaller buffer.

The smallest reproducer is:

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

int main(int argc, char **argv) {
    char foo[4];
    readlink("/run/current-system", foo, atoi(argv[1]));
    return 0;
}

Compile cc -O1 -D_FORTIFY_SOURCE=2 -o crash crash.c.

Call as either ./crash 4 (should not crash), ./crash 5 (should crash!), or FAKECHROOT_EXCLUDE_PATH=/dev:/proc:/sys:/nix/store/:/run fakechroot chroot . ./crash 4 (crashes, shouldn't.)

(Thanks @yorickvP for helping out!)

[kirillrdy]

The issue seems to be a bug in fakechroot; its shim of __readlinkat_chk seems to call the next readlinkat with a buffer size of FAKECHROOT_PATH_MAX-1, but with the buflen equivalent to the smaller buffer.

The smallest reproducer is:

#include <unistd.h>
#include <stdlib.h>
#include <stdio.h>

int main(int argc, char **argv) {
    char foo[4];
    readlink("/run/current-system", foo, atoi(argv[1]));
    return 0;
}

Compile cc -O1 -D_FORTIFY_SOURCE=2 -o crash crash.c.

Call as either ./crash 4 (should not crash), ./crash 5 (should crash!), or FAKECHROOT_EXCLUDE_PATH=/dev:/proc:/sys:/nix/store/:/run fakechroot chroot . ./crash 4 (crashes, shouldn't.)

(Thanks @yorickvP for helping out!)

@puckipedia thank you for taking time looking into this !

It looks like compiler bug to me

int main(int argc, char **argv) {
    char foo[4];
    readlink("/run/current-system", foo, 4);
    return 0;
}

FAKECHROOT_EXCLUDE_PATH=/dev:/proc:/sys:/nix/store/:/run fakechroot chroot . ./crash

doesn't crash

[puckipedia]

it's not a compiler bug; glibc with hardening enabled will pass a hidden parameter, the size of the buffer being written into, if known. If the maximum length passed into the syscall is larger than the statically determined size of the buffer, it bails out, knowing that the buffer would've been overflowed. It doesn't do this on static readlink calls because it can statically determine that the buffer can't overflow. fakechroot messes up handling this behaviour in a way that usually wouldn't matter, but that due to nixpkgs' defaults for hardening, shows up more often than other distros.

[risicle]

Indeed there's an old adage that's something like "if you think you've found a compiler bug, you haven't".

I'm interested in the relevance of this line:

https://github.com/dex4er/fakechroot/blob/b42d1fb9538f680af2f31e864c555414ccba842a/src/__readlink_chk.c#L25

[puckipedia]

That line is fine. This isn't entirely the platform to discuss the ins and outs of a bug in fakechroot, but i'll do it anyways:

The _FORTIFY_SOURCE define is needed to induce glibc into introducing the definition of __readlink_chk. The nextcall for __readlink_chk reuses the buflen as passed in by the program while reading into a buffer the size of FAKECHROOT_PATH_MAX-1. as FAKECHROOT_PATH_MAX-1 (bufsiz) > 4 (buflen), glibc assumes that the buffer was in fact only 4 bytes long, and crashes the program, as it seems someone just tried to overflow the buffer. The fix would be to just use FAKECHROOT_PATH_MAX-1 instead of buflen, as well as manually check for bufsiz > buflen (except when buflen == -1).

[risicle]

Of course, that makes sense.

[kirillrdy]

closing as this is fakechroot issue

[yorickvP]

We should probably submit a patch to fakechroot to fix this

[kirillrdy]

upstream issue dex4er/fakechroot#114