Skip to content

Fix potentially SPA-vulnerability #1341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

npt-1707
Copy link

Description
This PR fixes a security vulnerability in uECC_sign_with_k() that was cloned from micro-ecc but did not receive the security patch. The original issue was reported and fixed under kmackay/micro-ecc@1b5f5ce.
This PR applies the same patch to eliminate the vulnerability.

References
https://nvd.nist.gov/vuln/detail/CVE-2020-27209
kmackay/micro-ecc@1b5f5ce

@tencent-adm
Copy link
Member

tencent-adm commented Apr 21, 2025

CLA assistant check
All committers have signed the CLA.

@ynyyn
Copy link

ynyyn commented May 15, 2025

Turely, several critical dependencies used by mars/xlog are copied into the repository, with unclear source versions and increasingly outdated, posing potential maintenance risks in the future.

However, in this particular case, the changes from this PR may fail to effectively bring actual security fixes or improvements to mars/xlog as intended.

This is primarily due to two interesting facts:

1. There are multiple copies of micro-ecc in the mars repository

While the changes in this PR modify the latter one copy of micro-ecc, which belongs to decode_log_file_c_impl. This module was added to the repository later in recent years and is a C-based implementation of the log decryption, serving as a supplement to the original Python decryption script (likely for easier cross-platform migration). It is not integrated into the publicly released client/app (because the client would need the developer's private key to perform ECC decryption).

Thus, this PR should ideally also update (or even prioritize updating) the first copy of micro-ecc implementation (under mars/xlog/crypt).

However—

2. mars/xlog's use of micro-ecc does not involve the parts affected by CVE

The micro-ecc provides both ECDH and ECDSA functionalities. According to CVE-2020-27209, the security vulnerability in older versions of micro-ecc lies in the ECDSA operation, which is vulnerable to simple power analysis attacks, allowing an adversary to extract the ECC private key:

The ECDSA operation of the micro-ecc library 1.0 is vulnerable to simple power analysis attacks which allows an adversary to extract the private ECC key.

—ECDSA is a digital signature algorithm. It’s important to note that mars/xlog does not use ECDSA; it only uses ECDH key agreement to generate log encryption keys, which does not involve private key signing.

The encryption mechanism of mars/xlog can be described in detail as follows:

  1. The developer's public key is embedded in the program and released.
  2. The xlog dynamically generates a temporary key pair (public and private keys) at runtime.
  3. The xlog uses the newly generated temporary private key and the developer's public key to derive a symmetric encryption key, which is then used to encrypt the log data. The temporary public key is stored in the log chunk header.
  4. When the developer retrieves the log files later, they can derive the same symmetric key using their private key and then decrypt the log data.

It’s clear that this is not a private key signing process. Correspondingly, the client does not (and does not need to) have access to the developer’s private key—only the public key is required. Therefore, mars/xlog is not actually affected by the issue described in this CVE.


However, updating critical dependencies (including micro-ecc) to newer versions or implementations is always a good practice. And indeed mars does not do well in this regard.


确实 mars/xlog 所使用的几个关键依赖库都是复制到仓库的副本,来源版本号不详,并且年代逐渐久远,未来存在维护风险。

但就这个案例来说,此 PR 的修改可能并不能如愿给 mars/xlog 带来实质性的安全性修复或改进。

主要是由于存在这两点有趣的事实:

1. mars 仓库里有多份 micro-ecc 拷贝

而这个 PR 所修改的是后者那份 micro-ecc 拷贝,隶属于 decode_log_file_c_impl。这个模块是后来的这几年才被添加到仓库中的,它是一个基于 C 实现的一套日志解密库,可以把它认为是对原本 Python 解密脚本的补充,提供一个基于原生代码的实现(可能是方便跨平台迁移),并不会集成到对外发布的客户端版本中(因为客户端若需要利用 ECC 解密的话,需要获得开发者私钥。)

所以,此 PR 理应同时(或更应该)更新第一份 micro-ecc 拷贝实现。

然而——

2. mars/xlog 对 micro-ecc 的使用未涉及到存在 CVE 的部分

micro-ecc 同时提供 ECDH 和 ECDSA。根据 CVE-2020-27209 描述,旧版本 micro-ecc 所存在的安全隐患是 ECDSA 操作,容易受到 simple power analysis 攻击而被攻击者提取 ECC 私钥:

The ECDSA operation of the micro-ecc library 1.0 is vulnerable to simple power analysis attacks which allows an adversary to extract the private ECC key.

——ECDSA 是 数字签名 算法;需要注意,mars/xlog 并未使用 ECDSA,而是只使用了 ECDH 密钥协商 生成日志加密密钥,过程中并不涉及私钥签名。

mars/xlog 的加密机制可被详细论述为:

  1. 开发者公钥写入程序对外发布。
  2. xlog 程序在运行时动态生成一对临时公、私钥。
  3. xlog 程序使用新生成的临时私钥与开发者的公钥协商出对称加密密钥,此后用该对称加密密钥加密日志数据,并将临时公钥保存到日志数据分块头部。
  4. 后期开发者回收日志文件后,通过开发者私钥可协商出相同的密钥,从而解密日志数据。

可见,这不是一个私钥签名过程;相应地,客户端不拥有开发者私钥(也无需拥有),只需开发者公钥。因此 mars/xlog 事实上不受此 CVE 所指问题的影响。


但无论如何,更新包括 micro-ecc 在内的关键依赖库的版本或代码实现总是一个好的实践。在这方面 Mars 确实做得不够。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants