Firejail: --build broken

[reinerh]

I just noticed that the --build feature seems to be broken currently:

$ firejail --build cat ~/.bashrc
Error: the output file needs to be owned by the current user.
Error: cannot run the sandbox

[SkewedZeppelin]

Unable to reproduce on Fedora or Arch built from 9c44e7a437475f1bd93039873cfbcaf61d5a4e92

[chiraag-nataraj]

Unable to reproduce on Debian sid+experimental built from df3f0b618e237cd2fcd74ea7a3ef398fbe290ca9

[reinerh]

oh, interesting... thanks. will investigate why it fails here.

[reinerh]

Okay, it was an issue on my side...
I still had a file /tmp/firejail-trace owned by the root user.
And invoking --build with my normal user failed because it wanted to write trace output there as well.

But there still seems to be a different issue. By just creating a file /tmp/firejail-trace you can deny other users from using certain firejail functionality which requires writing to that file.
firejail doesn't even clean it up. So when you run firejail --build ls, and afterwards run the same command as another user, it will not be successful.
It's a small denial of service, if you can prevent other users from doing stuff.

Anyone else able to reproduce that?

[SkewedZeppelin]

By just creating a file /tmp/firejail-trace you can deny other users from using certain firejail functionality which requires writing to that file.

I can reproduce that.

[chiraag-nataraj]

Hmm. I created /tmp/firejail-trace (sudo touch /tmp/firejail-trace) and yet I was still able to use firejail --build successfully...
[edit] never mind, I can reproduce.

[reinerh]

So root is the owner of your file and you are running firejail as normal user?
When you add --debug, do you see --output=/tmp/firejail-trace?

[chiraag-nataraj]

@reinerh When I ran it the first time, I suspect touch didn't actually do anything (since the files already existed). When I cleaned up from previous runs and repeated the steps, I could reproduce the issue.

[Vincent43]

Perhaps instead of touch /tmp/firejail-trace it should use mktemp /tmp/firejail-trace-XXXXXX.

[chiraag-nataraj]

Working on it.

[chiraag-nataraj]

Also @Vincent43, the touch stuff was just me testing to see if I could reproduce. In the C code, there was a hard-coded filename, which is what I ended up changing (so same idea, just the C version).

[reinerh]

@chiraag-nataraj Are you also cleaning up the files again when they are no longer needed? I couldn't find an unlink in your commit.
Because that was also a reason why this bug occured.

[chiraag-nataraj]

@reinerh I'm currently not doing that, but since a unique file is created each time it's run, the bug should be fixed. I can also modify it to clean up after itself, but I wasn't sure if we wanted to keep the files around so that they could be inspected later?

[reinerh]

I think cleaning up would be better, as they are no longer needed after the profile is built, and it would only fill /tmp with trash.

[chiraag-nataraj]

:+1: I'll put that in.