Add 2024 audit report and accompanying documentation

6 files changed, 341 insertions, 5 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index faedf0eccc..2ec9b48edb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -68,18 +68,22 @@ Line wrap the file at 100 chars.                                              Th
 ### Security
 - Remove invalidly set up alternative stack for fault signal handlers on unix based systems.
   This prevents potential stack overflow and heap memory corruption.
-  Fixes audit issue `MLLVD-CR-24-01`.
+  Fixes audit issue [`MLLVD-CR-24-01`].
 - Remove/disable not signal safe code from fault signal handler on unix based systems.
-  Fixes audit issue `MLLVD-CR-24-02`.
+  Fixes audit issue [`MLLVD-CR-24-02`].
 
 #### Windows
 - Fix issue where the installer would allow any executable named `taskkill.exe` in the working
-  directory to run as admin. This fixes audit issue `MLLVD-CR-24-06`.
+  directory to run as admin. This fixes audit issue [`MLLVD-CR-24-06`].
 
 #### Linux
 - Prevent attackers able to send ARP requests to the device running Mullvad from figuring out
-  the in-tunnel IP. Fixes 2024 audit issue `MLLVD-CR-24-03`.
+  the in-tunnel IP. Fixes 2024 audit issue [`MLLVD-CR-24-03`].
 
+[`MLLVD-CR-24-01`]: audits/2024-12-10-X41-D-Sec.md#MLLVD-CR-24-01
+[`MLLVD-CR-24-02`]: audits/2024-12-10-X41-D-Sec.md#MLLVD-CR-24-02
+[`MLLVD-CR-24-03`]: audits/2024-12-10-X41-D-Sec.md#MLLVD-CR-24-03
+[`MLLVD-CR-24-06`]: audits/2024-12-10-X41-D-Sec.md#MLLVD-CR-24-06
 
 ## [2024.7] - 2024-10-30
 This release is identical to 2024.7-beta1.
diff --git a/audits/2024-11-30-X41-D-Sec-Audit-Report-v1.pdf b/audits/2024-11-30-X41-D-Sec-Audit-Report-v1.pdf
new file mode 100644
index 0000000000..ed35ea964c
--- /dev/null
+++ b/audits/2024-11-30-X41-D-Sec-Audit-Report-v1.pdf
diff --git a/audits/2024-12-10-X41-D-Sec-Audit-Report-v2.pdf b/audits/2024-12-10-X41-D-Sec-Audit-Report-v2.pdf
new file mode 100644
index 0000000000..76bbc848d6
--- /dev/null
+++ b/audits/2024-12-10-X41-D-Sec-Audit-Report-v2.pdf
diff --git a/audits/2024-12-10-X41-D-Sec.md b/audits/2024-12-10-X41-D-Sec.md
new file mode 100644
index 0000000000..e14914c296
--- /dev/null
+++ b/audits/2024-12-10-X41-D-Sec.md
@@ -0,0 +1,327 @@
+# 2024-12-10 - X41 D-Sec security assessment of the Mullvad VPN app
+
+Four people from [X41 D-Sec](https://x41-dsec.de/) (also referred to as just X41 in this document)
+performed a penetration test and source code audit of the Mullvad VPN app for a
+total of 30 person-days between 2024-10-23 and 2024-11-28.
+The audit report was handed over to Mullvad on 2024-11-30.
+
+The security assessment included all five supported platforms, and they were audited
+at the following versions respectively:
+
+* Desktop (Windows, Linux, macOS) - [2024.6](../CHANGELOG.md#20246---2024-10-23)
+* iOS - [2024.8](../ios/CHANGELOG.md#20248---2024-10-14)
+* Android - [2024.8-beta1](../android/CHANGELOG.md#android20248-beta1---2024-10-21)
+
+
+Quoting the key conclusions of the report:
+
+> A total of six vulnerabilities were discovered during the test by X41. None were rated as having
+a critical severity, three as high, two as medium, and one as low. Additionally, three issues
+without a direct security impact were identified.
+
+> Overall, the Mullvad VPN Application appear to have a high security level and are well
+positioned to protect from the threat model proposed in this report. The use of safe coding and
+design patterns in combination with regular audits and penetration tests led to a very hardened
+environment.
+
+> In conclusion, the client applications exposed a limited number of relevant vulnerabilities.
+Mullvad VPN AB addressed them swiftly and the fixes were audited to be working properly.
+
+## Focus areas of audit
+
+The goal of the audit was to identify issues in the app that might affect the security
+or privacy of the user. In other words, to ensure that the app protects the user as
+described in the threat model we agreed upon together with X41. See chapter
+*2.1 Threat Model* in the report for details.
+
+The main focus areas where we asked X41 to spend most of their time included:
+
+* Can an outside attacker learn parts of the user identity, or cause the device to leak traffic?
+* Can an outside attacker cause the VPN application to crash or otherwise malfunction?
+* Can a low-privileged local user use the Mullvad VPN app as a vector to escalate privileges?
+
+## Read the report
+
+The audit report was first delivered to Mullvad on 2024-11-30. It was a really good report
+as is, but together with X41 we identified some sentences which were slightly unclear.
+X41 then handed us a revised final report on 2024-12-10. Both versions are hosted on X41's
+website:
+* [2024-11-30 first version](https://www.x41-dsec.de/static/reports/X41-Mullvad-Audit-Final-Report-2024-11-18.pdf)
+* [2024-12-10 final version](https://www.x41-dsec.de/static/reports/X41-Mullvad-Audit-Public-Report-2024-12-10.pdf)
+
+We host both versions of the report in this repository:
+* [2024-11-30-X41-D-Sec-Audit-Report-v1.pdf](./2024-11-30-X41-D-Sec-Audit-Report-v1.pdf)
+* [2024-12-10-X41-D-Sec-Audit-Report-v2.pdf](./2024-12-10-X41-D-Sec-Audit-Report-v2.pdf)
+
+## Overview of findings
+
+This chapter will present a summary of each finding along with Mullvad's response to them.
+For more details about the finding, please read the report.
+
+A total of six vulnerabilities were discovered during the test by X41. None were rated as
+having a critical severity, three as high, two as medium, and one as low.
+Additionally, three issues without a direct security impact were identified.
+
+Mullvad implemented fixes for four of the issues during the audit, and released a new version
+of the app on the affected platforms around the time when we were handed the final audit report.
+
+### __MLLVD-CR-24-01__: Signal Handler Alternate Stack Too Small (Severity: High)
+<a id="MLLVD-CR-24-01"></a>
+
+The alternative stack configured for the fault signal handler in `mullvad-daemon` was too small.
+Since there were no guard page or other stack overrun protections in place, this could lead to
+the signal handler reading and writing beyond the allocated stack, leading to potential heap
+corruption and undefined behavior. This affected Android, Linux and macOS.
+
+We fixed this by just not setting up an alternative stack at all for our fault signal handler.
+These changes were done first in [PR #7137] and later updated in [PR #7172].
+The signal handler will now run on the default stack, or the altstacks set up by Rust and Go.
+All of which guard against stack overflows.
+
+The fix for this issue is included in version [`2024.8`] for desktop and version
+[`2024.9-beta1`] for Android. These versions were released just as the audit was completed.
+
+We agree with the conclusion from X41 that this vulnerability is not trivial to exploit, but if
+exploited it would be severe. Due to the low exploitability and the fact that this issue has
+been present for multiple years without any practical issues surfacing, we decided to not
+immediately mark existing apps as unsupported, but to release a fixed app version as soon
+as the audit was complete. We still recommend users on the affected platforms to
+upgrade to the latest version of the app at their earliest convenience.
+
+### __MLLVD-CR-24-02__: Signal Handler Uses Non-Reentrant Safe Functions (Severity: High)
+<a id="MLLVD-CR-24-02"></a>
+
+The fault signal handler in `mullvad-daemon` called functions which are not signal
+safe. This could cause undefined behavior, or worst case, be exploitable if the attacker
+was able to control enough of the program state and externally trigger a fault.
+This affected Android, Linux and macOS.
+
+More specifically, the signal handler had code which caused heap allocations. This included
+log string formatting and obtaining backtraces.
+A good description of [signal safety] can be found on the man page.
+
+We fixed this by re-implementing the signal logging using only signal safe code, and by not
+using the `log` Rust crate. We were not able to make backtrace rendering signal safe, but
+we still wanted the backtraces in debug builds. So we decided to keep the unsafe backtrace code,
+but disable it by default. Backtrace logging can be enabled by explicitly setting
+`MULLVAD_BACKTRACE_ON_FAULT=1`, or by compiling the program with debug assertions.
+
+The fix for this issue is included in version [`2024.8`] for desktop and version
+[`2024.9-beta1`] for Android. These versions were released just as the audit was completed.
+
+We are not aware of any way to maliciously or accidentally exploit or trigger this bug. This bug
+has been around for multiple years without any practical issues surfacing. So just like for
+`MLLVD-CR-24-01` above, we decided to not release any quick patch release immediately, but instead
+wait for the audit to finish and release fixes for all audit findings at the same time.
+
+### __MLLVD-CR-24-03__: Virtual IP Address of Tunnel Device Leaks to Network Adjacent Participant (Severity: Medium)
+<a id="MLLVD-CR-24-03"></a>
+
+The Linux kernel (and consequently Android) by default replies to ARP requests for any local
+target IP address, configured on any interface. This allows a network adjacent
+attacker (same local network) to learn the IP address of the VPN tunnel interface by
+sending an ARP request for every private IPv4 address to the device.
+
+This can be used by an adversary on the same local network to make a qualified guess if the device
+is using Mullvad VPN. Furthermore, since the in-tunnel IP only changes monthly, the adversary can
+also possibly identify a device over time.
+
+Linux and Android are the only affected operating systems. All other platforms we support only
+respond to ARP requests if the target IP is assigned to the interface where the ARP
+request arrives.
+
+On Linux we solved the issue by changing the kernel parameter [`net.ipv4.conf.all.arp_ignore`]
+to `2` whenever a VPN tunnel is established. This change was done in [PR #7141] and is included
+in the desktop app release version [`2024.8`].
+
+Android apps, including Mullvad VPN, do not have the permission to change kernel parameters such as
+`arp_ignore`. All Android devices that we know of are affected, as it is the default behavior of the
+OS. We have reported this issue [upstream to Google], and recommended that they change the kernel
+parameter to prevent the device from disclosing the VPN tunnel IP to the local network in this way.
+See the report for more details. We have added this Android issue to our document of [known issues].
+
+We don't consider this a high severity leak since the in-tunnel IP does not disclose a lot about
+the user. The IP is also automatically rotated every month, only making it a temporary identifier.
+However, Android users that are worried can log out and back in to the app, as this gives them
+a new tunnel IP.
+We are working on solutions that stops the in-tunnel IP from remaining the same over time.
+When this has been deployed, the issue will be gone on Android also.
+
+[known issues]: ../docs/known-issues.md#MLLVD-CR-24-03
+
+### __MLLVD-CR-24-04__: Deanonymization Through NAT (Severity: Medium)
+<a id="MLLVD-CR-24-04"></a>
+
+All UDP connections from a client to some service on the internet have a corresponding entry in the
+NAT table on the exit VPN relay the client is using. If a UDP packet arrives at the exit IP
+of the VPN relay with the source IP and port matching the peer internet service, and the
+destination port matches the random port assigned to the connection in the NAT table,
+then the relay will forward the packet in the tunnel, to the client.
+An attacker who can both spoof UDP source addresses and observe the client's tunnel traffic,
+can learn if the client is talking to a specific service or not.
+They can do this by sending packets of a specific size, and observe tunnel packets of that size
+(plus VPN tunnel header size) being sent to the client. Or they can send floods of packets and
+observe the volume of VPN traffic to the client increase. This affects all platforms and versions
+of the app.
+
+The attack would be hard to carry out. First of all the attacker would need to be able to send UDP
+packets with spoofed source IPs. Many network providers prevent this, but not all of them.
+The attacker would also need to be able to observe the client's tunnel traffic. On top
+of this, the attacker would also need to send large volumes of data to carry out the attack.
+If the attacker knows what VPN relay IP address the client exits through, they would need to send
+tens of thousands of packets before hitting the correct destination port, that match the
+relay's NAT table entry. Since every Mullvad relay has multiple exit IPs, and each client is
+assigned a random IP, the attacker would need to figure out what exit IPs the relay has,
+and repeat the above brute force method on all of them. Moreover, if the client uses
+multihop, the attacker can't easily infer what exit VPN relay the client uses. The attacker
+must then perform the above brute force attack against every exit IP of every Mullvad relay.
+All of this must be carried out in the somewhat short amount of time that the NAT table entry
+is active on the relay, meaning a time window of just a few minutes around when the client device
+communicates with the internet service.
+
+This issue is not specific to the Mullvad VPN service or our app. This is a general flaw in UDP.
+Since UDP is becoming a more common and important protocol due to http/3 and similar,
+Mullvad would love if it became the norm that all network providers performed UDP source address
+validation, as it would mitigate issues like this to a large extent.
+
+The [DAITA] feature can mitigate this attack to some extent. Since all packets are padded to the
+same size, and extra noise packets are injected, it becomes harder for the attacker to
+detect when their probing packet is forwarded to the client.
+
+Mullvad does not plan to actively mitigate this issue further in the app. The attack is already
+hard to carry out, and can be prevented further by enabling multihop and/or DAITA.
+Concerned users can also choose to avoid using UDP to communicate with sensitive services.
+
+### __MLLVD-CR-24-05__: Deanonymization Through MTU (Severity: Low)
+<a id="MLLVD-CR-24-05"></a>
+
+This attack is about how an attacker that can both observe a user's tunnel traffic and also
+manipulate internet traffic en route to the exit VPN relay of the user can potentially
+deanonymize the user. By adjusting the MTU of the traffic, delaying or dropping packets
+or cause traffic bursts in connections outside the tunnel, they can observe if the same
+traffic patterns occur on the encrypted tunnel traffic.
+With this information they can potentially infer if the connections belong to the
+user of the observed tunnel or not.
+
+Attacks like these are not specific to Mullvad VPN. The attack simply relies on core
+internet functionality and pattern matching. The threat model defined in the report
+makes it clear that it's virtually impossible to be fully protected against a very
+powerful attacker that can observe and manipulate internet traffic on a global scale.
+
+[DAITA] mitigates this attack to some extent by padding all packets to the same size
+and injecting noise in the tunnel. This makes it significantly harder for the
+attacker to detect the pattern they created in the tunnel.
+
+Mullvad's multihop feature makes this attack harder to carry out. Multihop hides
+the client's real IP from the exit VPN relay. If the attacker can observe and control traffic
+in and out of the exit VPN relay, they can perform the above attack. But if the client
+is using multihop, the attacker cannot see the real IP of the client. The attacker can
+deduce which entry VPN relay the client likely connects via, but they must then
+also be able to observe all traffic in and out of the entry VPN relay to find the IP
+of the client. Preventing attacks like these was one of the reasons why multihop
+was introduced, and is why Mullvad recommends using entry and exit relays from different
+hosting providers for the best protection.
+
+We think this kind of attack is not in the threat model of most users.
+However, we encourage everyone to consider their own situation and decide what they
+need to protect against.
+
+We agree with the severity rating being set to *low* on this issue, since it requires a powerful
+attacker and only provide them with heuristics to make qualified guesses about who the client is.
+
+### __MLLVD-CR-24-06__: Windows installer runs adjacent taskkill.exe (Severity: High)
+<a id="MLLVD-CR-24-06"></a>
+
+The Windows installer for the Mullvad VPN app invokes `taskkill.exe` in some places to kill
+processes as part of the install/upgrade procedure. Some of these invocations did not
+use absolute paths to the binary. This made Windows prefer any binary with that name,
+placed in the working directory (the directory where the installer is executed from)
+over the `taskkill.exe` shipped with the system. This issue only affects Windows.
+
+If the user was tricked into placing a malicious binary named `taskkill.exe` in the same directory
+as the Mullvad VPN app installer, the installer would execute the malicious code. Most installers
+are likely executed from the web browser's default download directory. This is also where
+any `taskkill.exe` file would end up, if the user had previously been tricked
+into downloading such file. Combining these facts makes the vulnerability not too unlikely or
+hard to carry out.
+
+Since the installer runs with administrator privileges, this vulnerability allows for
+privilege escalation. Given the impact of a compromise, and how relatively easy it is
+to trigger, we agree with the severity rating of *high*.
+
+The fix for this was to always specify the absolute path to `taskkill.exe`. This fix was
+made to the app in [PR #7225] and merged the same day the vulnerability was reported to us.
+
+The fix was released in version [`2024.8`]. Since the vulnerability only exists in the installer,
+and not the actual VPN application, we decided to not mark existing apps as unsupported or
+vulnerable. An already installed app is not affected by this.
+
+### __MLLVD-CR-24-100__: Publisher Not Set on Windows uninstaller
+
+The app's uninstaller binary for Windows has no publisher information set. This cause Windows
+to ask the user if they want to allow a program from an "Unknown publisher" to make changes
+to their system.
+
+This is not technically unsafe in any way, but it looks bad and can make the user doubt the
+authenticity of the application. If ignoring system warnings like this becomes the norm
+among users, it becomes easier to perform phishing attacks against them.
+We want users to pay attention to details around publisher signatures and identities as it makes
+them more resilient against phishing attempts and other malicious programs.
+
+We have started looking into adding proper publisher information to our uninstaller.
+
+### __MLLVD-CR-24-101__: Binary Hardening
+
+The third party tool [`binary-security-check`] reports that some of the binaries shipped
+with the Windows version of our app is missing some potential hardening. This does not
+constitute a direct security risk, nor indicate the presence of bugs in the software.
+This just means we have not enabled all possible techniques to prevent
+hypothetical present or future bugs from turning into exploitable security risks.
+
+We have started looking into what would be required for us to enable these
+binary hardening techniques and how much practical benefit it would be for our users.
+
+[`binary-security-check`]: https://github.com/koutheir/binary-security-check
+
+### __MLLVD-CR-24-102__: IOCTL Unrestricted Kernel Pool Memory Allocation
+
+The split tunneling kernel driver on Windows allowed the `mullvad-daemon` (and
+by extension any low-privileged process) to add an unrestricted number of processes
+to the list of processes excluded from the tunnel. This can cause system stability issues,
+since the kernel can end up allocating a lot of the available memory.
+
+This does not constitute a security risk (potential system instability not counted).
+As described in our [security documentation], the Mullvad app assumes all software and
+users on the local machine are trusted and not malicious. Any user is allowed to disable
+the VPN. This is by design and has been the case since the first version of the app.
+
+We will be updating the code to restrict the number of excluded processes to mitigate this
+issue. No special release will be made with just this fix, since it's not a problematic bug.
+The fix should be included in the next split tunnel driver version.
+
+## Last words
+
+Mullvad is very happy with the quality of the audit performed by X41 D-Sec.
+X41 managed to find issues in our code that previous audits missed, which shows that there
+is great benefit in having audits performed by different companies.
+This is not meant as criticism against the previous audit companies.
+The app is too big to realistically look into every aspect and detail in a few weeks.
+We have always had the explicit tactic to use a different third
+party auditor for every audit, to get different sets of eyes from people with different
+skills and mindsets every time.
+
+We would like to thank X41 D-Sec for their great security assessment and the nice
+collaboration we have had with you during the planning and execution stages of the audit.
+
+
+[PR #7137]: https://github.com/mullvad/mullvadvpn-app/pull/7137
+[PR #7141]: https://github.com/mullvad/mullvadvpn-app/pull/7141
+[PR #7172]: https://github.com/mullvad/mullvadvpn-app/pull/7172
+[PR #7225]: https://github.com/mullvad/mullvadvpn-app/pull/7225
+[DAITA]: https://mullvad.net/blog/daita-defense-against-ai-guided-traffic-analysis
+[`net.ipv4.conf.all.arp_ignore`]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
+[`2024.8`]: ../CHANGELOG.md#20248---2024-12-04
+[`2024.9-beta1`]: ../android/CHANGELOG.md#android20249-beta1---2024-11-27
+[upstream to Google]: https://issuetracker.google.com/issues/378814597
+[signal safety]: https://man7.org/linux/man-pages/man7/signal-safety.7.html
+[security documentation]: ../docs/security.md
diff --git a/audits/README.md b/audits/README.md
index 737e0032d8..5d0773a471 100644
--- a/audits/README.md
+++ b/audits/README.md
@@ -10,3 +10,4 @@ performed on this app so far:
 * [2018-09-24 - Assured and Cure53](./2018-09-24-assured-cure53.md)
 * [2020-06-12 - Cure53](./2020-06-12-cure53.md)
 * [2022-10-14 - Atredis](./2022-10-14-atredis.md)
+* [2024-12-10 - X41 D-Sec](./2024-12-10-X41-D-Sec.md)
diff --git a/docs/known-issues.md b/docs/known-issues.md
index 28d53b6af5..2e9a2887d1 100644
--- a/docs/known-issues.md
+++ b/docs/known-issues.md
@@ -229,6 +229,7 @@ it very difficult to properly secure them.
 
 
 ### Android exposes in-tunnel VPN IPs to network adjacent attackers via ARP
+<a id="MLLVD-CR-24-03"></a>
 
 By default the kernel parameter [`arp_ignore`] is set to `0` on Android. This makes the device reply
 to ARP requests for any local target IP address, configured on any interface. This means that any
@@ -251,8 +252,11 @@ tunnel IP.
 
 #### Timeline
 
-* November 6, 2024 - Auditors reported this issue on Linux and Android, later classified as `MLLVD-CR-24-03`.
+* November 6, 2024 - Auditors from X41 D-Sec reported this issue as part of the [2024 app audit].
+  The issue was given the identifier [`MLLVD-CR-24-03`].
 * November 14, 2024 - We reported the issue [upstream to Google].
 
 [`arp_ignore`]: https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
+[2024 app audit]: ../audits/2024-12-10-X41-D-Sec.md
+[`MLLVD-CR-24-03`]: ../audits/2024-12-10-X41-D-Sec.md#MLLVD-CR-24-03
 [upstream to Google]: https://issuetracker.google.com/issues/378814597