Bug 1218896 (CVE-2023-32190)

This is an embargoed bug. This means that this information is not public.

Please do NOT:
- talk to other people about this unless they're involved in fixing the issue
- make this bug public
- submit this into OBS (e.g. fix Leap/Tumbleweed) until this bug becomes
  public. This means that the security team removed the EMBARGOED tag from
  the bug title after we verified that there's already information about
  this bug publicly available. If you find such information yourself and
  the bug is still embargoed please contact us

Your primary responsibility is to apply a fix for this issue.
Here is some guidance on openSUSE package maintenance:
- https://en.opensuse.org/openSUSE:Package_maintenance
- https://en.opensuse.org/openSUSE:Maintenance_update_process

You need to submit AFTER the bug became public, to the current openSUSE
Leap codestreams, and to the devel project of your package.

The security team will then take the following steps:
- We wait for your submission and package them into an incident for QA
  testing. The QA tester might reach out to you if they find issues with
  the update.
- If QA doesn't find any issues, we publish the updates.

You can contact us at:

* IRC: irc.suse.de #security
* Do NOT use Slack or any non-SUSE hosted messaging services
* Email: security-team@suse.de

Internal CRD: 2024-02-16 or earlier

The chmod was added to fix a bug. Removing it would reintroduce the bug, so it's obviously not desirable.

Your POC implies acting as "nobody", however my understanding is that logging as this user isn't possible. The only way would be to su to that user from the root account. But if the attacker has already gained access to the root account then exploiting this bug is not longer needed.

So I think the chmod is only an issue if the attacker has already exploited another bug in some service running as ${RUN_UPDATEDB_AS} to be able to perform arbitrary actions as that user?

So far the fix for bsc#1188933 was only pushed into Factory.

However, there are customer PTFs and pending maintenance update requests (SUSE/openSUSE).

In this case a symbolic link is considered a "regular file". That's the reason why the test case 'if [ -f ".../mlocate.db" ]' is true and executes the 'chmod 644' command.

What we could do here is to check explicitly for a symbolic links. The bash has the test case '-h' or '-L' for checking this.

       -h file
              True if file exists and is a symbolic link.
       -L file
              True if file exists and is a symbolic link.

111 # If database permissions are wrong (see bsc#1188933), fix them
112 if [[ -f "%{_localstatedir}/lib/mlocate/mlocate.db" &&
113       ! -h "%{_localstatedir}/lib/mlocate/mlocate.db" ]]
114 then
115         chmod 644 "%{_localstatedir}/lib/mlocate/mlocate.db"
116 fi

There is currently no switch for 'chmod' to prevent the "follow symlink" issue directly.

We could do that, but that would be racy. The symlink could be created between the test and the call to chmod.

> There is currently no switch for 'chmod' to prevent the "follow symlink"
> issue directly.

That's the first thing I looked for, but indeed, unfortunately, it doesn't exist. It might be worth implementing, for future use cases if not for this bug. FWIW, BSD's chmod has option -h which changes the mode of the link itself instead of following it. I don't think Linux allows the mode of symbolic links to be anything other than 777.

(In reply to Jean Delvare from comment #3)
yes, this allows escalation of privileges from nobody. For the attack someone else needs to have a way to act as nobody. But as this user is used sometimes as a "sandboxing" mechanism this is reasonable. Nobody must not be able to gain root privileges.

(In reply to Jean Delvare from comment #6)
it is indeed racy. Changing the directory owner before running chmod is safe and since this only happens during updates shouldn't cause issues. After the chmod run the directory ownership can be changed back

And yes, having -h for chmod would be handy. I'll check if I can submit this

I can't see how changing the directory owner alone would solve the problem. Presumably the attacker has created the symlink already, so it's too late, changing the directory owner won't prevent the symlink from being followed by chmod.

Changing the directory would only help prevent the race mentioned in comment #6. So we would change the owner of the directory to root and then check that mlocate.db isn't a symlink before calling chmod.

However this solution introduces another race condition: if the mlocate database update service is running while we are doing that, and RUN_UPDATEDB_AS is set to nobody, there is a risk that the update fails because it can't write the file to the now root-owned directory.

I thought hard about a solution to that problem that would solve it completely without creating any race condition, nor having any side effect, but couldn't find any.

My first idea was to run chmod as "nobody" (su nobody -c 'chmod 644 "%{_localstatedir}/lib/mlocate/mlocate.db"'). This fixes the security issue, but only fixes the file permissions if RUN_UPDATEDB_AS was set to "nobody" (which isn't the default). It's still valuable as customers changing UMASK to a more strict setting (which is what triggers bug #1188933 in the first place) are more likely to also want a more restrictive mlocate database. Still, these are separate settings, and it is perfectly valid to change one without changing the other. So I don't feel too comfortable with this approach.

I also considered triggering a database update as part of the post-install script of mlocate, as this would fix the file permissions as a side effect. However, this is a heavy I/O operation which is normally timed during the night. Running it as part of a system update may add the I/O burden at an inappropriate time. It is also questionable to perform such a costly operation only to fix the permissions of a file.

All in all I start wondering if the cure isn't worse that the problem the chmod call was trying to solve. After all, it's only addressing a transitory issue that will solve itself overnight anyway. I think I prefer consistently having to wait for the next mlocate update (something we could simply document in a TID), than introducing a race or an inconsistent behavior.

Note that most Tumbleweed users must have installed the update by now anyway, so the file permissions are already fixed on their system. So we are really only discussing SLE customers, and the ones which have reported the bug already received a PTF so the file permissions are fixed for them too.

So I take back what I wrote in comment #3, simply removing the troublesome chmod call could be acceptable and possibly the best solution. I originally added it as the icing on the cake, in an attempt to make customer experience as smooth as possible, but if it's going to cause more trouble than is worth, we can do without.

(In reply to Jean Delvare from comment #9)

> All in all I start wondering if the cure isn't worse that the problem the
> chmod call was trying to solve. After all, it's only addressing a transitory
> issue that will solve itself overnight anyway. [...]
> 
> So I take back what I wrote in comment #3, simply removing the troublesome
> chmod call could be acceptable and possibly the best solution.

Yes, I agree with your sentiment. I too believe that we should fix this security issue by removing the `chmod` call. It's not perfect, but the number of users affected by the issue that this `chmod` addresses is probably small. We could also mention the permission problem (and how to remedy it) in the patchinfo of the upcoming SLE updates and in the TID we're going to publish for this CVE. I think that's a decent solution.

(In reply to Jean Delvare from comment #8)
It doesn't solve the problem fully, but after you changed the permissions you can operate safely in the directory and not worry about the user attacking you. You then can check for symlinks and not worry about TOCTOU issues.

And yes, this might interfere with a db update, but since this only happens in cases where the update runs in parallel with a package update this would seem acceptable to me. That should be rare and cure itself upon the next db update.

If we can drop the chown call that would be the best solution from a security POV. No chmod call == no attack angled.

(In reply to Jean Delvare from comment #9)
> but only fixes the file permissions if RUN_UPDATEDB_AS was set to "nobody"
> (which isn't the default).

For correctness, I must amend this claim of mine. On my laptop, RUN_UPDATEDB_AS is an empty string, and as I don't remember ever touching /etc/sysconfig/locate, I assumed this was the default. However, the current content of /usr/share/fillup-templates/sysconfig.locate sets RUN_UPDATEDB_AS to "nobody" by default, so this would be the default now for a fresh system installation.

All done, closing.