-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Insecure Temporary Files #338
Comments
I've opened issue USBGuard/usbguard#277 for USBGuard. |
Thank you very much, Topi, for taking time to report this, initiating (Cat is out of the bag by now, and GitHub cannot make the issues private TBH, this got me really worried that I should have pursued the initial Regarding the possible quick workaround at the client side, commented What works with Linux kernel 4.19+ (or perhaps through distro backports)
This is deemed a good enough immediate workaround where supported Patch following the suggestion above to come once several things gets |
In addition to
It would be a good idea to mention the security reporting link on Github front page README.markdown and also ClusterLabs libqb page. The security page itself mentions encrypted email but no email address is actually given. |
We don't use FIFOs (anywhere in the cluster stack but some resource |
Regarding the hints, yes, we were likely resting on our laurels, hence If I can speak for the vendor side, for instance Red Hat has this: Distro representatives surely scan distros list and would notify Last but not least, you can get the idea of who is maintaining this All these could be done in parallel, though I agree it is perhaps |
FWIW. there are many tightly coupled considerations regarding the |
Wouldn't adding |
wferi: I'm working on a patch that adds O_EXCL to the files and also adds a random number suffix to the names. That should help the worst of the problems. |
Adding just |
PR#339 |
Alternative is #343 Btw. when this double-problem gets sufficiently fixed, following can be EDIT: only if filesystem sockets are enforced and build-time configured |
release v1.0.4 includes O_EXCL when opening files and uses files in mkdtemp to create them in directories under /dev/shm |
Hi, do we need a CVE ID for this? |
I'm still waiting for one. |
Since I upgraded to libqb 1.0.4 Pacemaker experiences problems:
Connection to the CIB is impossible:
Here's my
|
Hmm I'm not sure what's happening there, it worked in testing. I'm off tomorrow so it'll have to wait till Thursday at least. sorry. I did notice that the directories are 0700 and should be 0770 (so there's a chmod missing in the patch) but that doesn't fix it so there's something else happening too. |
Strace gives me this:
The path on the last one seems to be mangled compared to the previous libqb version:
|
Strange enough now I get a different listing now:
|
@vvidic, that first trace looks like memory corruption, can you reproduce it? |
Not always but now it seems to appear again:
|
Just for the info this is libqb0 1.0.4-1 with pacemaker 2.0.1-2 all from Debian unstable. Also uploading the full strace in case someone can read the binary protocol: crm_mon.strace.gz |
@vvidic Strangely, I can't reproduce this corruption. It might be a mishandled/ignored error on the client side. I can see lots of interesting things in the pacemaker startup logs, though, like
It's worth noting that we configure libqb |
@wferi Yes, it seems to behave a bit different every time I boot the VM or restart pacemaker. I also see these errors in the logs so it might be that after the error pacemaker gets some random data from memory into this string? Installing the libqb0_1.0.4-2_amd64.deb from your repo fixes the problem for me too. |
@vvidic The sign mistake made pacemaker-based send back a positive |
@chrissie-c Have you seen PR#347? I think those signs should be fixed as well for correct propagation of errors. |
@wferi Apologies, I did not. That looks better to me, I'll put the chown back in there (for corosync), and give it some testing |
I was thinking of
Is there something I've missed? |
I think |
ahh fair enough I hadn't realised that. So the first of your two options above makes best sense to me now then |
OK, I replaced those commits. Now Pacemaker works and all directories under
Is this expected? |
Thanks. The perms are consistent with 1.0.3. |
True, so I suppose it's OK then. |
Not precisely. The libqb tests are a constant thorn on my side. Mostly they're failing with rather than anything 'real'. It's one of the, many, reasons I got rid of the linker sections for 2.0. But we'll have to live with that for a little while yet. |
This problem (as initially disclosed here) has been assigned Once again, thanks for the report. |
I'm currently investigating whether version 0.11 in Debian 8 "Jessie" is vulnerable. Am I correct that the following commit is the fixing commit for CVE-2019-12779 6a4067c |
Should #347 also be applied? |
Can we say that setting both |
On 17/06/19 01:45 -0700, wferi wrote:
Can we say that setting both `fs.protected_hardlinks` and
`fs.protected_symlinks` to 1 fully mitigates this problem? (That's
the only supported configuration since Debian jessie.)
I'd be very cautious to call that an unconditional closing of the
gap, since there are effectively more vectors pertaining this problem.
AFAICT, `fs.protected_*links` would only resolve "powers roundtrip"
attacks, like said overwriting of privileged files in case a privileged
IPC server is tricked into that (talking about the situation prior to
the `O_EXCL` fix).
OTOH, it doesn't fix eavesdropping and possibly and/or active mangling
with the channel (most likely with DoS effect, crafted diversion is
unlikely [for more shared resources, namely semaphores, involved], but
wasn't disproved either) venues coming with the vulnerability at hand.
…--
Jan (Poki)
|
On 18/06/19 07:17 -0700, Gao,Yan wrote:
On 6/14/19 11:47 PM, Jan Pokorný wrote:
> On 14/06/19 14:02 -0700, Markus Koschany wrote:
> > Should #347 also be
> > applied?
>
> Definitely, at least if you want to have the cluster stack functional.
I think it should be #349,
right?
my bad, was familiar enough ... hopefully this line of progress
over PR iterations (as commented on) naturally follows
Any reason why it doesn't seem to be in the master branch?
Ah, good spot, it wasn't intentional I am sure.
…--
Jan (Poki)
|
Hello all. I am working on an update of libqb in Debian 8. The libqb package in that Debian release is somewhat old, being at version 0.11.1. I have adapted what I believe are the three relevant commits from the 01_CVE-2019-12779_1_of_3.txt If these patches look correct, then I will proceed with updating the package in Debian 8. Otherwise, I will make whatever corrections or adjustments are needed. |
On Mon, Nov 11, 2019 at 07:54:07AM -0800, Roberto C. Sánchez wrote:
Hello all. I am working on an update of libqb in Debian 8. The libqb
package in that Debian release is somewhat old, being at version
0.11.1. I have adapted what I believe are the three relevant commits
I suppose you mean version 1.0.1-1 of the Debian package? Probably best
to contact wferi since he was working on libqb fixes for Debian.
…--
Valentin
|
@vvidic No, I am in fact referring to version 0.11.1-2 in Debian 8 "jessie," which is supported by the LTS team. |
No problem. When the versions are closer we oftentimes coordinate
between the LTS team and the stable security team. However, the
versions involved in this case are quite far apart.
|
What uses libqb in Debian 8?
I think you'd need the |
On Mon, Nov 11, 2019 at 03:10:57PM -0800, wferi wrote:
What uses libqb in Debian 8?
It would appear that nothing in the archive depends on it:
root@build01:/# apt-cache rdepends libqb0
libqb0
Reverse Depends:
libqb-dev
root@build01:/# apt-cache rdepends libqb-dev
libqb-dev
Reverse Depends:
root@build01:/#
I think you'd need the chown patch and at least the error handling fix
from [4]#349 as well. The buffer overflow parts may not apply to 0.11, I
can't check it now.
For the chown patch, I was not able to find a place where the change was
applicable. I do not recall why I excluded the error handling fix.
I will take another look.
|
@wferi I have taken another look and the reason I left out the
|
I see, shared memory IPC changed much since 0.11. I don't think it's worth spending effort on fixing this issue in Debian 8; anybody using it there must have used the 0.17 or 1.0 backport instead. Unfortunately jessie-backports is closed, so we can't do anything for them now. |
@wferi Thank you for the assessment. Your insight on this matter is
appreciated.
|
Yes, I think you'll have trouble backporting that patch to 0.11. libqb is only used for corosync 2.0 onwards so I'm not really surprised that it has no reverse dependancies |
I brought this up to the team and we are going ahead with dropping support for libqb in Debian 8 "jessie". Thank you to all for the help and guidance on this. |
Hi! Is my understanding correct that this issue was fixed by 6a4067c for releases >=1.9.0? |
@hartwork That's correct |
@chrissie-c thank you! 👍 |
Libqb creates files in world-writable directories (
/dev/shm
,/tmp
) with rather predictable file names (e.g./dev/shm/qb-usbguard-request-7096-835-12-data
in case of USBGuard). AlsoO_EXCL
flag is not used when opening the files. This could be exploited by a local attacker to overwrite privileged system files (if not restricted by sandboxing, MAC or symlinking policies).At least
O_EXCL
flag should be used. I'd also use more complex logic where files are created with unpredictable names (also usingO_TMPFILE
) and then possibly renamed to match file naming convention (if the protocol does not allow completely random file names). I would not use files for IPC.The text was updated successfully, but these errors were encountered: