What the developer wrote
Authored by SomberNight
logging: get_git_version: add timeout
note: this is currently only used by the crash reporter, on-demand when constructing a report
f321x said:
> Some git commands can be really slow on certain filesystems setups, this is for example the output on a virtio9p mount in a vm:
>
> user@debian-vm:~/code/electrum$ time git describe --always --dirty
> 4.8.1-62-gab813173cd
>
> real 0m2.614s
> user 0m0.109s
> sys 0m0.196s
What changed, and why it matters
This commit adds a 5-second timeout and a small cache to a helper that runs the 'git describe' command when Electrum's crash reporter builds a diagnostic report. The change mainly prevents the crash reporter from hanging on slow filesystems. A code comment also flags that the helper searches the system path for a program named 'git', which could theoretically be abused if an attacker controls the program search path, but the commit does not claim this is an exploitable vulnerability.
No immediate action required. Treat as routine hardening. If reviewing further, verify that crash-reporter subprocess helpers sanitize/validate the executable path and do not rely on attacker-controllable PATH or working directory.
Security signals we found
Addition of subprocess timeout to prevent indefinite hang
Addition of result caching to reduce repeated external command invocations
Inline developer comment questioning PATH/PWD executable lookup attack surface
Evidence from the diff
The patch modifies electrum/logging.py’s get_git_version(). It adds @lru_cache so the result is computed once per process, and adds timeout=5 to the subprocess.check_output() call. The author notes the function is only used by the crash reporter on demand. A new inline comment raises a question about $PATH/$PWD lookup of an executable named ‘git’ as potential attack surface, but no exploit or security fix is described. The change is defensive hardening against slow git invocations rather than a remediation of a disclosed security issue.
Changed components
electrum/logging.pyget_git_version()crash reporterInspect captured patch +7 / −1
### electrum/logging.py
@@ -5,6 +5,7 @@
import logging
import logging.handlers
import datetime
+from functools import lru_cache
import sys
import pathlib
import os
@@ -353,11 +354,16 @@ def describe_os_version() -> str:
return platform.platform()
+@lru_cache # result unlikely to change while running. though "-dirty" could technically change
def get_git_version() -> Optional[str]:
dir = os.path.dirname(os.path.realpath(__file__))
try:
+ # note: this searches $PATH and perhaps $PWD for an executable named "git". attack surface?
version = subprocess.check_output(
- ['git', 'describe', '--always', '--dirty'], cwd=dir)
+ ['git', 'describe', '--always', '--dirty'],
+ cwd=dir,
+ timeout=5, # seconds
+ )
version = str(version, "utf8").strip()
except Exception:
version = NoneWhy this scored 17/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.