Closed Bug 1682101 (CVE-2021-29949) Opened 4 years ago Closed 4 years ago

Thunderbird may use OTR library using a filename that isn't distributed

Categories

(Thunderbird :: Security, defect)

Product:
Thunderbird
Component:
Security
Type:
defect

Tracking

(thunderbird_esr78+ fixed, thunderbird88 fixed)

Status:
RESOLVED FIXED
Milestone:
89 Branch
Tracking Flags:
Tracking Status
thunderbird_esr78 + fixed
thunderbird88 --- fixed

People

(Reporter: vupt.bka, Assigned: KaiE)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1682101 - Prefer loading of the distributed OTR library. r=mkmelin
48 bytes, text/x-phabricator-request
wsmwk
: approval-comm-beta+
wsmwk
: approval-comm-esr78+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/87.0.4280.88 Safari/537.36

Steps to reproduce:

  1. Summary:
    The newest version of Mozilla Thunderbird Product has 02 vulnerability about dll hijacking and lead attackers to perform an arbitrary code execution by loading a unverify dll.
  • Vulnerability Process: thunderbird.exe (Local at : "%Program Files (x86)%\Mozilla Thunderbird\thunderbird.exe")
  • The dll name Hijacked: otr-5.dll, otr.5.dll
  1. Steps To Reproduce:
    When user runs thunderbird.exe 02libraries with name otr-5.dll and otr.5.dll at 1.Summary are loaded by thunderbird.exe with a relative name that makes it exposed to DLL hijacking. When an application or a service is starting on Windows it looks for the used DLLs in order to function properly. If these DLLs don’t exist or the software code is developed in an insecure way (DLL’s are called without using a fully qualified path) then it’s possible to load and execute a malicious DLL file.

It should be noted that when an application needs to load a DLL it will go through the following order:
The directory from which the application is loaded
C:\Windows\System32
C:\Windows\System
C:\Windows
The current working directory
Directories in the system PATH environment variable

  1. These steps below will show how to escalate privilege:
  • Create a fake dll with name "otr-5.dll". Its code just calls the notepad.exe process by cmd.exe. (with user - not admin)
  • Copy the fake dll "otr-5.dll" to a folder which has writeable permission with user. I choose folder "C:\Python27" (It is contained in "Directories in the system PATH environment variable")
  • Open thunderbird.exe, my fake otr-5.dll is called first, and the notepad.exe process runs.

Actual results:

Attacker can run all malicious commands on victim computer with this issue whenever user opens thunderbird.exe.

Expected results:

Thunderbird.exe should load the truth dlls with verified digital on the truth path.

Component: Untriaged → Installer

Wayne: why Installer? It appears we cannot ship OTR (incompatible license?) so we can't "fix" the bug by installing it ourselves into a place where we can expect it.

This broad path search looks intentional.

Tuan Vu: In my own instance of Thunderbird none of the chat functionality is active, because I never set it up. Did you set up chat and tell Thunderbird that you wanted to use OTR? Or is it looking for OTR even though you didn't? We shouldn't be opening up this risk for users who don't even want that functionality, but for those that do I'm not sure how we'll engineer a good balance between "making it work" and opening up this path injection risk if the OTR install directory happens to be later in the path than some world-writable directory.

[Suggestion off the top of my head: on first set-up do the path look-up, and when we find OTR remember that specific path in a preference. Users could edit it if they ever move OTR. I assume you've already rejected "Tell Thunderbird where OTR is installed" as unusable like used to be done for GPG in Enigmail.]

Flags: needinfo?(vupt.bka)
Flags: needinfo?(vseerror)

I don't disagree with you, I just wanted to get it out of untriaged component. I leave it to others to make a better choice.

Flags: needinfo?(vseerror)

Looks a lot like bug 1530726. It's really the "current working directory" piece check that's problematic. The others should be admin-only writable paths on Windows, and if an attacker has written a malicious DLL to one of those directories then you're already in trouble.

Official Thunderbird builds ship with libotr shared libraries for some time now. Since the broad search path is there to accommodate Linux distributions really who prefer to link to system libraries for their builds of Thunderbird, I suggest we mitigate by only checking the application directory for libotr on Windows.

Dear Team,
First, we just need user permission to write some malicious files on those paths
Second, I detected this issue happenned when i run thunderbird (don’t care using OTR or not)
Thanks Team!

Flags: needinfo?(vupt.bka)

(In reply to Rob Lemley [:rjl] from comment #3)

I suggest we mitigate by only checking the application directory for libotr on Windows.

Sounds good. I wouldn't think a "system otr" dll is a thing on windows.

Rob, if you want to make the patch, please go ahead and ask for review. If you want me to make the patch, please assign the bug to me.

Attached patch Bug1682101-OTR.patch (obsolete) — — Splinter Review

This is what I came up with; it does the job, but I'm not particularly crazy about it. Since we have libotr, librnp, and a couple of others, is it worthwhile trying to unify the ctypes.open loading code? I looked around for inspiration but didn't see anything.
I think some simple unittests should be added as well.

Attachment #9193173 - Flags: feedback?(kaie)

(In reply to Rob Lemley [:rjl] from comment #7)

Created attachment 9193173 [details] [diff] [review]
Bug1682101-OTR.patch

This is what I came up with; it does the job, but I'm not particularly crazy about it. Since we have libotr, librnp, and a couple of others, is it worthwhile trying to unify the ctypes.open loading code? I looked around for inspiration but didn't see anything.
I think some simple unittests should be added as well.

Dear Mr Rob Lemley,
I think it's ok.
You should load those libs from your program path.
So this is a vuln, please patch it soon.

Magnus - I guess I missed Kai last week, what do you think of this approach? What about the other libs like RNP?

Flags: needinfo?(mkmelin+mozilla)

Looks ok to me, but sorry, no real insight into what's best here. Don't we always use our own RNP?

Flags: needinfo?(mkmelin+mozilla)

For RNP, it first checks the application directory (and it will be there 99% of the time) but if that fails, there is a fallback to looking in standard system library locations.
https://searchfox.org/comm-central/rev/c4336ca990fc2c70be11c7554f24bdaa40936013/mail/extensions/openpgp/content/modules/RNPLib.jsm#40-47

And there is also potentially libgpgme: https://searchfox.org/comm-central/source/mail/extensions/openpgp/content/modules/GPGMELib.jsm#40-47

Honestly, this is not a Thunderbird issue. It's an insecure OS that sticks a potentially user-writable path in the search list when looking for libraries... but since that's not going to change....

If you look at the three examples I give, we have pretty much the same code copy-and-pasted to find and load these libraries. Rather than fix this for libotr and have to deal with the same problem tomorrow with libgpgme, would it make sense to write a generic loader that handles this?

Looks like you'd need quite a bit of parametrization, which usually isn't good for readability. Maybe.

Dear Team,
Do you have any information about this issue?

Flags: needinfo?(rob)

The attached patch should mitigate the issue, I'd like Kai to weigh in on my approach.

I thought I'd updated this bug... Thunderbird should probably not be looking for libraries that it might not find. I want to get libotr building "right" so that it's always there then we don't have this kind of issue. That work is going slowly though (bug 1634836) so landing this fix will mitigate.

Libgpgme loading is off by default and librnp will be there with a default build.

Flags: needinfo?(rob) → needinfo?(kaie)

(In reply to Daniel Veditz [:dveditz] from comment #1)

It appears we cannot ship OTR (incompatible license?) so we can't "fix" the bug by installing it ourselves into a place where we can expect it.

Dan, this is a misunderstanding on your side.
Thunderbird does ship OTR code by default.
We distribute LGPL shared modules.

(In reply to Magnus Melin [:mkmelin] from comment #5)

(In reply to Rob Lemley [:rjl] from comment #3)

I suggest we mitigate by only checking the application directory for libotr on Windows.

Sounds good. I wouldn't think a "system otr" dll is a thing on windows.

Where does the existing code attempt to load from CWD ?
And where does the suggested patch prevent that?

Flags: needinfo?(kaie)

Ok, never mind, now I get it.

The code can either use an absolute path (which will limit loading from there), or using a filename w/o path, which can load from anywhere the search path configuration defines.

With your patch, your first load attempt uses useSystem=false, which causes us to construct an absolute path, using the application directory.

If that fails, on Windows and macOS you will NOT do a second attempt to load from elsewhere.

Because building OTR libs isn't yet done as part of the build, this effectively disables macOS developers from using homebrew, right?

Only on on other platforms (Linux etc.) you attempt a fallback to load files from system locations.

So, to clarify what our bug is:

The first filename we attempt to load on Windows is a filename that we don't use in the distribution. That causes us to look for the file in alternative directories.

The remedy for the bug is to always start our search with the expected filename that we distribute. With that, the fallback is usually avoided. In order to exploit the bug, an attacker would need to delete a library from the software installation directory.

I think the same strategy should be used for loading the RNP library.

However, for optional libraries like GPGME we need to allow the flexible approach, trying multiple filenames from multiple locations.

Status: UNCONFIRMED → NEW
Component: Installer → Security
Ever confirmed: true

Dear Team,
Do you have any information about this security issue?
When will this vuln fixed?
Thanks!

Have you any update for this issue?

Flags: needinfo?(kaie)

Dear Team,
plz reply about this issue soon.

Rob, you had not yet responded to my question in comment 17, in particular:

Because building OTR libs isn't yet done as part of the build, this effectively disables macOS developers from using homebrew, right?

Flags: needinfo?(rob)

Comment on attachment 9193173 [details] [diff] [review]
Bug1682101-OTR.patch

[deleted - incorrect statement]

Flags: needinfo?(kaie)
Attachment #9193173 - Flags: feedback?(kaie) → feedback-

Sorry, my comment 24 is wrong. I'm deleting it.

In my opinion, we should allow a fallback to system directories on all platforms.

In other words, let's start by loading the expected filename from Thunderbird's directory.
If that fails (expected to fail for OTR library in developer builds ), allow loading from system directories.

We should do the same for RNP. A distribution might decide to not ship RNP, and load it from elsewhere.

Maybe worth treating Windows differently? Doesn't seem like there's ever a need for a fallback there.

What about Thunderbird developers who build on Windows? They won't get an OTR library as part of the build. They need to copy a downloaded library into their path.

Flags: needinfo?(rob)
Attachment #9193173 - Attachment is obsolete: true
Assignee: nobody → kaie

Maybe have a special pref they need to set?

Summary: A security issue on your Mozilla Thunderbird Product (the newest version 78.5.1) → Thunderbird may use OTR library using a filename that isn't distributed

(In reply to Magnus Melin [:mkmelin] from comment #29)

Maybe have a special pref they need to set?

Do you agree that we should be consistent across all platforms?

Let's say we require a pref for loading OTR from outside the Thunderbird distribution directory, and the default is false.
Then it would still work in all binaries that Thunderbird distributes.
However, if there is any Linux/BSD/etc. distribution that doesn't ship OTR as part of the Thunderbird package, then that distribution will fail to work. The distribution will have to change their package to set the new pref to a nonstandard value "load from the system".

Maybe this could be acceptable, but the question is, do we really need it?

What are we trying to achieve?

We want to achieve that the binary in the Thunderbird distribution directory is loaded with the highest priority. We can achieve that without a pref.

If we don't add a pref, if we always allow a fallback to load from outside the Thunderbird directory, we should still be safe - because the attacker would be required to have write access to the Thunderbird directoy, and be able to delete the primary library.

I have already tested Linux locally. We should test what console messages are printed on Windows and macOS using this patch. If the patch is correct, we should get one "trying to load" message only.

I started a try build with the patch here:
https://treeherder.mozilla.org/jobs?repo=try-comm-central&revision=059f0d9224550850cfb0761609e1a85c360f79c2

I can test on my Windows VM (win10 x64).

We have a meta bug to flag distros if they need to change something. Unfortunately, they won't see this security bug anyway. I'm on the side of not using a pref.

What if we drop the SOVERSION from the final libname? libotr.so.5 -> libotr.so? That makes this code simpler at the expense some changes to the build script. I should be able to do that easily enough.

See bug 1700240 - libotr.dll for Windows, libotr.so for Linux, and libotr.dylib for macOS.

See Also: → 1700240

I have tested this patch and confirm it's working.

On Windows, first attempt is for libotr-5.dll and succeeds.
On macOS, first attempt is for libotr.5.dylib, which is the filename we ship.
So it should be ok to land this patch.

Loading didn't work on macOS for comm-central, because of bug 1700349, which seems like a new regression.

Rob, we probably want to backport this patch to esr78 - but I wouldn't want to backport a library file rename to esr78.
So, this means, we should probably ship this patch as is, and backport.

Your new suggestion isn't bad, but it means we'll have to change this code again.

Target Milestone: --- → 89 Branch

https://hg.mozilla.org/comm-central/rev/7a0a62f2aeea9ddb7c604b9e8b1cb2fb99a917d2

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Comment on attachment 9210769 [details]
Bug 1682101 - Prefer loading of the distributed OTR library. r=mkmelin

[Approval Request Comment]
Regression caused by (bug #): no
User impact if declined: risk of loading an incorrect library
Testing completed (on c-c, etc.):
Risk to taking this patch (and alternatives if risky): low

Attachment #9210769 - Flags: approval-comm-beta?

Dear Team,
When will the new version published?

It will have to go through beta. Follow this bug. Probably in 78.10 in about a month.

tracking-thunderbird_esr78: ? → +

Thanks.
Can we assign a CVE ID or have any bounty for this bug?

Thunderbird doesn't have a bounty program.
I guess there will be a CVE.

Oh what a pity. I saw it on scope of bounty :(

(In reply to Kai Engert (:KaiE:) from comment #35)

Rob, we probably want to backport this patch to esr78 - but I wouldn't want to backport a library file rename to esr78.
So, this means, we should probably ship this patch as is, and backport.

Works for me.

(In reply to Magnus Melin [:mkmelin] from comment #41)

Thunderbird doesn't have a bounty program.
I guess there will be a CVE.

Why has not bounty program for Thunderbird? I can't find this information anywhere.
Thunderbird is a valid product of Mozilla.
Thanks.

The bug bounty program is for Mozilla Corporation Firefox products only: https://www.mozilla.org/en-US/security/client-bug-bounty/
Thunderbird is its own entity, under the Mozilla Foundation.

(In reply to Magnus Melin [:mkmelin] from comment #45)

The bug bounty program is for Mozilla Corporation Firefox products only: https://www.mozilla.org/en-US/security/client-bug-bounty/
Thunderbird is its own entity, under the Mozilla Foundation.

Oh i see it said: "Mozilla manages two different bug bounty programs. One focuses on Firefox and other Mozilla applications and the other covers our websites and services."
And on the hackerone, thunderbird is also on scope.

It specifically says "Firefox, Firefox for Android, or Firefox for iOS as released by Mozilla Corporation "

(In reply to Tuan Vu Pham from comment #46)

...
And on the hackerone, thunderbird is also on scope.

https://hackerone.com/mozilla?type=team is incorrect/out of date. Someone should correct it.

"Client Reward Guidelines

The bounty for valid critical client security bugs will be $3000 (US) cash reward and a Mozilla T-shirt. The bounty will be awarded for sec-critical and sec-high severity security bugs that meet the following criteria:

Security bug is present in the most recent main development (i.e., Aurora, Beta or EarlyBird, and nightly mozilla-central releases) or released versions of Firefox, Thunderbird, Firefox for Android, or in Mozilla services which could compromise users of those products, as released by Mozilla Corporation."

It also cites https://www.mozilla.org/en-US/security/bug-bounty.html which no longer exists.

(In reply to Wayne Mery (:wsmwk) from comment #48)

(In reply to Tuan Vu Pham from comment #46)

...
And on the hackerone, thunderbird is also on scope.

https://hackerone.com/mozilla?type=team is incorrect/out of date. Someone should correct it.

"Client Reward Guidelines

The bounty for valid critical client security bugs will be $3000 (US) cash reward and a Mozilla T-shirt. The bounty will be awarded for sec-critical and sec-high severity security bugs that meet the following criteria:

Security bug is present in the most recent main development (i.e., Aurora, Beta or EarlyBird, and nightly mozilla-central releases) or released versions of Firefox, Thunderbird, Firefox for Android, or in Mozilla services which could compromise users of those products, as released by Mozilla Corporation."

Yes, so i just think Mozilla has bug bounty program with thunderbird because nowhere said not :D

I wonder which security severity rating should be assigned to this issue.
https://wiki.mozilla.org/Security_Severity_Ratings/Client

In order to exploit this issue, it isn't sufficient to simply download a file. The file must be copied to a directory in the execution PATH.

That is, anyone trying to abuse this issue needs to have access to the victim's name, must prepare a library with the correct name, and must have permission to copy it to one of the directories listed in PATH (or have permission to change the PATH variable).

Given that access to the victim's machine is required, I'm not sure if this qualifies as a security issue.

Is this simply a correctness fix?

Flags: needinfo?(tom)

(In reply to Kai Engert (:KaiE:) from comment #51)

I wonder which security severity rating should be assigned to this issue.
https://wiki.mozilla.org/Security_Severity_Ratings/Client

In order to exploit this issue, it isn't sufficient to simply download a file. The file must be copied to a directory in the execution PATH.

That is, anyone trying to abuse this issue needs to have access to the victim's name, must prepare a library with the correct name, and must have permission to copy it to one of the directories listed in PATH (or have permission to change the PATH variable).

Given that access to the victim's machine is required, I'm not sure if this qualifies as a security issue.

Is this simply a correctness fix?

Oh, your product didn't check the loaded dll, it drives to load an arbitrary code. I sure that it is a security issue.
What do you think whenever your product run, a malware is called and attacker compromised your system?

How does the malware get placed on the computer and into the executable directory?

(In reply to Kai Engert (:KaiE:) from comment #53)

How does the malware get placed on the computer and into the executable directory?

It is infection step, attacker use this issue for execution step.
And attacker use it to hide from defense system to detect because the malware is loaded by a signed process.

https://attack.mitre.org/techniques/T1574/001/
From mitre attack

I will work to correct the Hacker One profile - I didn't know that even existed.

My opinion on severity would be sec-low or sec-moderate. Not higher, certainly, because you need to be on the machine to begin with. But at least sec-low because in some configurations at least a differently- or lower-privileged process/user could use it to get code execution in Thunderbird and steal data. Maybe Dan has an opinion.

Flags: needinfo?(tom) → needinfo?(dveditz)

https://attack.mitre.org/techniques/T1574/001/
From mitre attack(In reply to Tom Ritter [:tjr] (ni? for response to sec-[advisories/bounties/ratings/cves]) from comment #56)

I will work to correct the Hacker One profile - I didn't know that even existed.

My opinion on severity would be sec-low or sec-moderate. Not higher, certainly, because you need to be on the machine to begin with. But at least sec-low because in some configurations at least a differently- or lower-privileged process/user could use it to get code execution in Thunderbird and steal data. Maybe Dan has an opinion.

Thanks. What about before you work to correct? Has any bounty for this bug?

Regressions: 1700506

Thanks. What about before you work to correct? Has any bounty for this bug?

We are not responsible for what HackerOne puts on their site. Our actual bounty program is published at https://www.mozilla.org/en-US/security/bug-bounty/ and has been there for longer than HackerOne has existed. The details change from time to time, and currently our client bounty is focused on the Firefox family of products (desktop and mobile).

Flags: needinfo?(dveditz)
Attachment #9210769 - Flags: approval-comm-esr78?

requires follow-up fix from bug 1700506

Comment on attachment 9210769 [details]
Bug 1682101 - Prefer loading of the distributed OTR library. r=mkmelin

[Triage Comment]
Approved for beta ...

(In reply to Kai Engert (:KaiE:) from comment #59)

requires follow-up fix from bug 1700506

Flags: needinfo?(justdave)
Attachment #9210769 - Flags: approval-comm-beta? → approval-comm-beta+

I'm assuming I got NIed so I could uplift it, but it seems Kai has done that already. If that wasn't what you needed from me, feel free to NI again.

Flags: needinfo?(justdave)

Comment on attachment 9210769 [details]
Bug 1682101 - Prefer loading of the distributed OTR library. r=mkmelin

[Triage Comment]
Approved for esr78

Attachment #9210769 - Flags: approval-comm-esr78? → approval-comm-esr78+

Dear Team,
Is this patch released?

Yes, in Thunderbird 78.9.1 and above.

Yeah, is this bug a security problem?
I have not seen it on the security advisor of thunderbird.

Theoretically, a small one yes, so good to fix. In practice, not very useful for an attacker who would have to have been able to put the dll in one of these system dirs. If he could do that, there are just easier ways to do more, in easier ways than tricking Thunderbird to load it.

Alias: CVE-2021-29949
Group: mail-core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: