Bug 1156767 (CVE-2020-7040) - VUL-0: CVE-2020-7040: storeBackup: /tmp/storeBackup.lock default lock file location opens up a local DoS attack vector
Summary: VUL-0: CVE-2020-7040: storeBackup: /tmp/storeBackup.lock default lock file lo...
Status: RESOLVED FIXED
Alias: CVE-2020-7040
Product: SUSE Security Incidents
Classification: Novell Products
Component: Incidents (show other bugs)
Version: unspecified
Hardware: Other Other
: P3 - Medium : Normal
Target Milestone: ---
Assignee: Security Team bot
QA Contact: Security Team bot
URL: https://smash.suse.de/issue/250970/
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-14 12:08 UTC by Matthias Gerstner
Modified: 2024-10-07 11:41 UTC (History)
2 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthias Gerstner 2019-11-14 12:08:55 UTC
+++ This bug was initially created as a clone of Bug #1150555

A finding from the cron job audit of storeBackup is that storeBackup by
default uses a lock file in /tmp/storeBackup.lock which opens up a DoS attack
vector for unprivileged local users.

If an unprivileged user simply does this:

$ echo 1 >/tmp/storeBackup.lock

then possibly configured system backups won't be executed, because storeBackup
thinks that an instance is already running.

Furthermore there's a race condition involved allowing a symlink attack.
storeBackup first performs a stat() then an lstat() on /tmp/storeBackup.lock
and only then opens it for creating. Thus if an unprivileged attacker wins
this race condition then files can be created or overwritten. This way a
system can be broken or if additional conditions are met it might even allow
to escalate privileges in some way.
Comment 2 Matthias Gerstner 2019-11-14 12:45:37 UTC
In attachment 824140 [details] there's a first suggested patch to change the default
lockfile location to /var/lock. Since /var/lock is only accessible to root,
both attack vectors should be addressed.

Let's keep the bug private for the moment. I've sent an email to the single
upstream author but my hopes aren't very high that there will be a timely
reaction.

The DoS against storeBackup itself would not justify a CVE assignment but the
race condition allowing to overwrite system files does. I will request one
from Mitre as soon as we'll publish this finding.
Comment 3 Jan Ritzerfeld 2019-11-14 14:52:32 UTC
(In reply to Matthias Gerstner from comment #2)

First, many thanks for your audit! I've looked at the source code and can confirm the findings.

> In attachment 824140 [details] there's a first suggested patch to change the
> default lockfile location to /var/lock. Since /var/lock is only accessible to
> root, both attack vectors should be addressed.

I don't have permissions to see this patch, but I guess that the patch either adds "-L /var/lock/storeBackup.lock" as an option or puts "lockFile=/var/lock/storeBackup.lock" into the storebackup.config.default template?

However, I think the race condition could be fixed by simply using sysopen() instead of open() in fileDir.pl, i.e., something like:
sysopen(FILE, $lockFile, O_WRONLY | O_CREAT | O_EXCL)

> Let's keep the bug private for the moment.

Okay. I first thought about opening a bug but I saw that you protected this one here.

> I've sent an email to the single
> upstream author but my hopes aren't very high that there will be a timely
> reaction.

At least, last year, there was activity in 
https://savannah.nongnu.org/support/?group=storebackup

> The DoS against storeBackup itself would not justify a CVE assignment but

I agree.

> the
> race condition allowing to overwrite system files does. I will request one
> from Mitre as soon as we'll publish this finding.

Yes, this one is a serious problem.
Comment 4 Matthias Gerstner 2019-11-14 15:18:02 UTC
(In reply to suse@bugs.jan.ritzerfeld.org from comment #3)
> First, many thanks for your audit! I've looked at the source code and can
> confirm the findings.

Good. Thank you for looking into this.

> I don't have permissions to see this patch, but I guess that the patch
> either adds "-L /var/lock/storeBackup.lock" as an option or puts
> "lockFile=/var/lock/storeBackup.lock" into the storebackup.config.default
> template?

Ah I'm not sure why you can't see it. I've marked the attachment private so
that it doesn't leak to the public at this time. Since you're part of this bug
you *should* be able to see it. Anyways the patch looks like this:

```
-my $lockFile = '/tmp/storeBackup.lock';   # default value
+my $lockFile = '/var/lock/storeBackup.lock';   # default value
```

for the three files bin/storeBackup.pl, bin/storeBackupDel.pl and
bin/storeBackupUpdateBackup.pl. I can also send you the patch vie email if you
like.

> However, I think the race condition could be fixed by simply using sysopen()
> instead of open() in fileDir.pl, i.e., something like:

This could be an additional protection measure. However lockfiles don't belong
in /tmp anyways. Changing the default will break storeBackup for non-root
users, but they can still pass an explicit lockfile path. To maintain backward
compatibility the patch could also choose a different default path depending
on the user that calls it. For example placing the lockfile in
/run/user/$UID/storeBackup.lock could be an approach.

> > Let's keep the bug private for the moment.
> 
> Okay. I first thought about opening a bug but I saw that you protected this
> one here.

You wanted to open a bug here in Bugzilla or upstream somewhere?

> At least, last year, there was activity in 
> https://savannah.nongnu.org/support/?group=storebackup

Okay then maybe there's hope. Let's give upstream a chance to react on this
before we publish it. I will then write a posting on the oss-security mailing
list so that other distros will get informed, too.
Comment 5 Jan Ritzerfeld 2019-11-17 20:27:13 UTC
(In reply to Matthias Gerstner from comment #4)
> (In reply to suse@bugs.jan.ritzerfeld.org from comment #3)
> [...] 
> Ah I'm not sure why you can't see it. I've marked the attachment private so
> that it doesn't leak to the public at this time.

Good idea.

> Since you're part of this bug you *should* be able to see it.

I suspect that I cannot see it because attachments have no assignee. And I can only see this bug because I'm the assignee.

> [...]
> > However, I think the race condition could be fixed by simply using sysopen()
> > instead of open() in fileDir.pl, i.e., something like:
> 
> This could be an additional protection measure.

Hmm, at least the symlink attack will fixed entirely. By a "one-liner". Not?

> However lockfiles don't belong in /tmp anyways.

Actually, /tmp has been the location of lock files for decades:
https://www.tldp.org/LDP/Linux-Filesystem-Hierarchy/html/tmp.html
But you are right, I don't see how the local DoS could be prevented if the lock file was still written to /tmp.

> Changing the default will break storeBackup for non-root
> users, but they can still pass an explicit lockfile path. To maintain backward
> compatibility the patch could also choose a different default path depending
> on the user that calls it. For example placing the lockfile in
> /run/user/$UID/storeBackup.lock could be an approach.

Nonetheless, this will alter the application's logic. The lock file in /tmp is a global lock ensuring that at most one application instance will ever run at any time. This will no longer hold if we move the lock to user specific directories.  The application can store backups of different sources in subdirectories of the same "root" destination directory to perform deduplication across all these different backups. Thus, I thought the global lock was an explicit design decision to prevent corruption by concurrent access by all means. However, I finally found this statement by the application's author:
"If two different users run backups, I think it's better to use different lock files." https://savannah.nongnu.org/bugs/?39915
So, I'm actually fine with this approach in case someone changes every perl file using /tmp, document this new behavior in the help outputs, the man pages, and the German and English PDFs (how BTW?). I'm not sure whether this is worth the effort comparing to the one-liner above...

> [...] 
> You wanted to open a bug here in Bugzilla or upstream somewhere?

I thought about opening a bug report upstream when I got the bugzilla notification of this bug here, IIRC this bug here was still public then.

> > At least, last year, there was activity in 
> > https://savannah.nongnu.org/support/?group=storebackup
> 
> Okay then maybe there's hope. Let's give upstream a chance to react on this
> before we publish it. I will then write a posting on the oss-security mailing
> list so that other distros will get informed, too.

Many thanks, again.
Comment 6 Matthias Gerstner 2019-11-18 12:28:18 UTC
(In reply to suse@bugs.jan.ritzerfeld.org from comment #5)
> I suspect that I cannot see it because attachments have no assignee. And I
> can only see this bug because I'm the assignee.

I'll send the patches to you via e-mail then.

> > [...]
> > > However, I think the race condition could be fixed by simply using sysopen()
> > > instead of open() in fileDir.pl, i.e., something like:
> > 
> > This could be an additional protection measure.
> 
> Hmm, at least the symlink attack will fixed entirely. By a "one-liner". Not?

Yes specifying correct open flags will fix the symlink attack.

> > However lockfiles don't belong in /tmp anyways.
> 
> Actually, /tmp has been the location of lock files for decades:
> https://www.tldp.org/LDP/Linux-Filesystem-Hierarchy/html/tmp.html
> But you are right, I don't see how the local DoS could be prevented if the lock file was still written to /tmp.

Yeah maybe from a standardization point of view but not from a security point
of view ;-). Especially when the root user is involved.

> Nonetheless, this will alter the application's logic. The lock file in /tmp
> is a global lock ensuring that at most one application instance will ever
> run at any time. This will no longer hold if we move the lock to user
> specific directories.

That is true. However the use case of protecting an instance of e.g. user root
against an instance of some other non-privileged user or even different
non-privileged users against each other is a bit strange. A non-privileged
user will not have the privileges to do a system backup, for example.

> The application can store backups of different sources in subdirectories of
> the same "root" destination directory to perform deduplication across all
> these different backups.

This would again, from a security point of view, be strange. It would mean
different users with different privileges will all have access to a shared
backup folder. Of course it doesn't mean that such setups don't exist.

> "If two different users run backups, I think it's better to use different lock files." https://savannah.nongnu.org/bugs/?39915
> So, I'm actually fine with this approach in case someone changes every perl
> file using /tmp, document this new behavior in the help outputs, the man
> pages, and the German and English PDFs (how BTW?). I'm not sure whether this
> is worth the effort comparing to the one-liner above...

It's the cleaner solution in any case. It fixes not only the symlink issue but
also the DoS issue. Being able to suppress a system backup running in the root
user's context is pretty serious in my opinion.
Comment 7 Matthias Gerstner 2019-11-26 09:04:28 UTC
I've got a short reply from the upstream maintainer Hein-Josef Claes by now.
He acknowledges the security issue. But he's got little time and can't tell
us when he'll be able to distribute a new version.

Given this statement I think I will request a CVE for this from Mitre, we
should choose the most sane bugfix for the moment and publish the finding on
the oss-security mailing list for others to become aware.
Comment 8 Matthias Gerstner 2020-01-14 12:57:06 UTC
CRD: 2020-01-17

Communication with the upstream maintainer continued to be slow. I've setup a
time limit for 2020-01-17. Upstream informed me that a new release is in
preparation and under individual testing and that it should be finished by mid
of January.

My plan is to publish this finding next week, no matter what upstream does. So
please can you commit fixes on Friday or shortly after and also submit
maintenance updates? Thank you!
Comment 9 Jan Ritzerfeld 2020-01-14 20:50:51 UTC
(In reply to Matthias Gerstner from comment #8)
> [...]
> My plan is to publish this finding next week, no matter what upstream does.
> So please can you commit fixes on Friday or shortly after and also submit
> maintenance updates? Thank you!

I'll commit my fix to Archiving:Backup on Friday and submit a request for Factory.
Should I create the maintenance request for Leap 15.1 in parallel or wait until my submit request for Factory is accepted? Because IIUC the latter is required for the bug fix to "be reflected in Package Hub 15":  
https://en.opensuse.org/openSUSE:Backports_Package_Submission_Process#Packager_workflow_for_Package_Hub_15
Comment 10 Matthias Gerstner 2020-01-15 08:49:47 UTC
I've requested a CVE for this finding from Mitre. Thanks to the new process
with the Mitre form, the CVE will not become immediately public, only when we
add a public reference to it.
Comment 11 Matthias Gerstner 2020-01-15 09:09:39 UTC
(In reply to suse@bugs.jan.ritzerfeld.org from comment #9)
> Should I create the maintenance request for Leap 15.1 in parallel or wait
> until my submit request for Factory is accepted? Because IIUC the latter is
> required for the bug fix to "be reflected in Package Hub 15":

I'm not quite sure actually. It might be enough to submit them all in one go
and the maintenance people might be satisfied with the patch being in
submission to Factory. Or they require that Factory actually accepted the
change.

I'd simply submit them all and see what happens.
Comment 12 Matthias Gerstner 2020-01-16 11:53:17 UTC
The upstream author now sent me a storeBackup-3.5.1 tarball over an untrusted
channel. This tarball contains ~2.000 lines of diff. Still no isolated patch
available from upstream. The upstream tarball sources don't have updated
versions yet. This process is not very professional for the year 2020.

I suggest we stick to the isolated patch we agreed upon. As soon as upstream
version 3.5.1 is distributed via official channels you can consider packaging
the update.
Comment 13 Jan Ritzerfeld 2020-01-16 21:01:14 UTC
(In reply to Matthias Gerstner from comment #11)
> I'd simply submit them all and see what happens.

Yes, since Factory is at least two commits ahead the patch for Factory won't apply in any mbrach. :)
Comment 14 Jan Ritzerfeld 2020-01-16 21:02:51 UTC
(In reply to Matthias Gerstner from comment #12)
> The upstream author now sent me a storeBackup-3.5.1 tarball over an untrusted
> channel. This tarball contains ~2.000 lines of diff. Still no isolated patch
> available from upstream. The upstream tarball sources don't have updated
> versions yet. This process is not very professional for the year 2020.

Well, at least upstream is alive.

> I suggest we stick to the isolated patch we agreed upon. As soon as upstream
> version 3.5.1 is distributed via official channels you can consider packaging
> the update.

I'll do. Thank you!
Comment 15 Matthias Gerstner 2020-01-20 13:16:09 UTC
I'm publishing this bug, please submit fixes to the devel project / Factory and maintenance updates now. Thank you!
Comment 16 Swamp Workflow Management 2020-01-20 19:10:06 UTC
This is an autogenerated message for OBS integration:
This bug (1156767) was mentioned in
https://build.opensuse.org/request/show/765898 Factory / storeBackup
Comment 17 Swamp Workflow Management 2020-01-20 23:20:06 UTC
This is an autogenerated message for OBS integration:
This bug (1156767) was mentioned in
https://build.opensuse.org/request/show/765940 15.1+Backports:SLE-15+Backports:SLE-15-SP1 / storeBackup
Comment 18 Matthias Gerstner 2020-01-21 09:31:49 UTC
I've published about this finding on oss-sec [1].

[1]: https://seclists.org/oss-sec/2020/q1/20

I've also added this as a public reference in the Mitre CVE form to publish
the CVE entry in their database.
Comment 19 Swamp Workflow Management 2020-01-28 17:15:03 UTC
openSUSE-SU-2020:0119-1: An update that fixes one vulnerability is now available.

Category: security (moderate)
Bug References: 1156767
CVE References: CVE-2020-7040
Sources used:
openSUSE Leap 15.1 (src):    storeBackup-3.5-lp151.5.3.1
openSUSE Backports SLE-15-SP1 (src):    storeBackup-3.5-bp151.4.3.1
openSUSE Backports SLE-15 (src):    storeBackup-3.5-bp150.3.3.1
Comment 20 Camila Camargo de Matos 2024-10-07 11:41:33 UTC
All affected and currently supported packages contain a fix for this issue. This bug, therefore, can be closed.