Kudelski Audit of Mantis
Kudelski Audit of Mantis
Kudelski Audit of Mantis
1 Summary 3
2 Introduction 4
3 Security issues 6
3.1 MANTIS-001: Insecure hash function used for integrity checks (MD5) . . . 6
3.7 MANTIS-007: Sensitive data not cleared before being garbage collected . 11
4 Observations 14
1
Security Audit IOHK
5 About 16
This document reports on our security audit of IOHK’s Ethereum Classic client “Mantis”,
a piece of software integrated in the Daedalus wallet application. The audit aimed to
discover security shortcomings in Mantis and help implement relevant mitigations.
We did not find any critical security issues, but report 3 medium-severity issues and
4 low-severity issues. Mantis otherwise shows good security engineering, with secure
defaults, defensive coding, and a clear code base that facilitates auditing.
The audit was lead by Dr. Jean-Philippe Aumasson, Principal Research Engineer at
Kudelski Security in January 2017, and took approximately 38 hours of work.
3
2 Introduction
This security audit covers IOHK’s Mantis Ethereum Classic client, with a focus on its
cryptographic components. The audit aimed to discover security shortcomings in the
protocols’ logic as well as implementation errors and potential for abuse from
malicious users. The main components reviewed include the keystore mechanism,
network protocol implementation (RLPx), transaction validators, Ethereum virtual
machine, and JSON RPC server.
The audit was performed by Dr. Jean-Philippe Aumasson, Principal Research Engineer,
with support from Yolan Romailler, Cryptography Researcher. Approximately 38 hours
of work were spent on the engagement.
• 3 medium-severity issues
• 4 low-severity issues
All the security issues have been investigated and fixed when possible.
4
Security Audit IOHK
Disclaimer Due to time constraints, proof-of-concepts were not created for all issues
found. Some findings may therefore be false alarms, or more benign than expected.
But in doubt, we prefer to report too many than too few issues.
Conversely, as with any security audit, we probably missed some issues affecting the
application. For example, yet unknown security issues in the Scala language, or subtle
protocol edge cases that could be discovered through formal verification, for example.
Furthermore, we did not review the code of third-party dependencies, but only
checked for known vulnerabilities or recently reported bugs therein. Bugs in
dependencies such as
github.com/json4s/json4s (JSON parsing),
github.com/suzaku-io/boopickle (serialization), or BouncyCastle (crypto, ASN1
parsers, etc.) could lead to exploitable vulnerabilities, since their code directly
processed untrusted data.
This section enumerates the alleged security issues discovered, presenting for each
issue a brief description aimed to a reader familiar with the code base, mitigation
proposals, as well as a severity rating in low/medium/high.
The Status subsection will report the feedback from developers (issue correctness,
mitigation implemented, etc.). When relevant, the Status subsection includes
references to pull requests. We have reviewed all the pull requests cited, to validate
that they correctly address the issue.
Severity: Medium
Description
Recommendation
6
Security Audit IOHK
Status
MD5 has been replaced with SHA-512 in the pull request “[EC-416] Change bootstrap
hashing to SHA512”1 .
Severity: Medium
Description
We didn’t do a complete review of RLPx, but in limited time we noticed that the RLPx
handshake is vulnerable to key-compromise impersonation (KCI) attacks: if a recipient’s
long-term key is compromised, an attacker can forge initiator messages and pretend to
be the initiator (by issuing a signature of a previous static-shared-secret using a fresh
ephemeral key). An attacker can even do the converse: forge a signature of a fresh
static-shared-secret with the same ephemeral private key as the legitimate initiator, by
picking a fresh nonce that is equal to the xor of the two shared secrets and the older
nonce.
The custom MAC-based construction looks okay, but is not a standard construction let
alone of provably secure one. In particular, the xoring of keys with nonces is a trick
that often led to insecure schemes (and that facilitates side-channel attacks).
1
https://github.com/input-output-hk/mantis/pull/385
Recommendation
Status
Mantis developers cannot do much about it, except adhere to the protocol to ensure
interoperability.
Severity: Medium
Description
Here a negative value of the gasUsed BigInt in a BlockHeader will not be rejected as
invalid:
Recommendation
Status
Pull request “[EC-418] Validation that gasUsed is positive number”2 fixed the issue with
the following patch to BlockHeaderValidator.scala:
Severity: Low
Description
Recommendation
Status
These two functions are actually only used for testing purposes.
Severity: Low
Description
Recommendation
Using the AES from the JDK (from javax.crypto) seems to be a safer option. For
example, OpenJDK’s latest versions will use AES-NI instructions if they are available,
see e.g. https://bugs.openjdk.java.net/browse/JDK-7184394. With such
implementation, AES will also be faster than with BouncyCastle’s implementation.
Status
Developers investigated the possible mitigations and noted that using AES-256 from
Java versions prior to 8u161 requires the installation of Java Cryptography Extension
(JCE), which would reduce the compatibility of the application. Given the threat model
of Mantis, we agreed that this issue is a low risk, and that a fix can be postponed.
Severity: Low
Description
Recommendation
Status
Severity: Low
3
https://github.com/input-output-hk/mantis/pull/387
Description
Sensitive data such as private keys can overwritten with zero or random data in order
to prevent other processes to read it when memory is reused (this can for example
be performed using the SecureZeroMemory() function in the Windows API). In Mantis,
sensitive data includes at least private keys and passphrases, and is not cleared before
being garbage collected.
Recommendation
Status
• Switching to mutable objects would not solve the issue, since libraries (such as
Circe and Akka) used for interfaces with third-party applications use immutable
objects.
• Even (e.g.) zeroized Char[] objects would not guarantee that all copies of the
sensitive data have been zeroized, because of the way generational garbage
collectors work.
• Reflection introduced complexity to the code and therefore potential bug. Too,
it could slow down the application, and again would not prevent the garbage
collector from creating copies of the object.
issues:
4
See https://highlyscalable.wordpress.com/2012/02/02/direct-memory-access-in-java/
This section reports various observations that are not security issues to be fixed.
14
Security Audit IOHK
Legacy primitives are also supported, but not used in the current version of the code:
PBKDF2-HMAC-SHA-256 and AES-CBC. If PBKDF2 is used, the number of iterations
should be high enough (say, 50000) to protect the passphrases.
Two implementations of Keccak (SHA-3) are used in Mantis: one from scrypt (in
Keystore) and one from BouncyCastle (in RLPx). It may be simpler to only use that
from BouncyCastle, unless there’s a specific reason not to.
The following dependency declared in build.sbt is not used in Mantis, and can be
omitted: "org.whispersystems" % "curve25519-java" sha1
"09091eb56d696d0d0d70d00b84e6037dcd3d98b6",
Kudelski Security is a division of Kudelski Group. For more information, please visit
https://www.kudelskisecurity.com.
Kudelski Security
route de Genève, 22-24
1033 Cheseaux-sur-Lausanne
Switzerland
16