shoko/jujutsu-skills
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

Add security audit to polymarket-browse review

Browse Source 
CRITICAL: Telegram bot token exposed in process command line
HIGH: HTML injection in Telegram messages
MEDIUM: Insufficient --search URL encoding
MEDIUM: No bounds check on --detail
MEDIUM: Potential DoS via large API response
LOW: Bare except: clauses
LOW: No API rate limiting

Includes fix recommendations and immediate actions for users.
This commit is contained in:
shoko
2026-03-25 09:27:28 +00:00
parent 4a33d6924e
commit 27c8cb3597
1 changed files with 288 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

289
skills/polymarket-browse/reviews/2026-03-25.md
View File

@@ -377,7 +377,294 @@ def test_converged_market_filter():
--- ---
## Appendix A: Longest Lines in browse.py (for targeted refactoring) ## 6. Security Audit
### 6.1 Severity Classification
| Severity | Meaning |
|----------|---------|
| CRITICAL | Immediate action required. Users should stop using the skill until fixed. |
| HIGH | Serious vulnerability. Fix as soon as possible. |
| MEDIUM | Moderate issue. Fix in normal development cycle. |
| LOW | Minor issue. Fix when convenient. |
---
### 6.2 CRITICAL: Telegram Bot Token Exposed in Process Listings
**Location:** Lines 598-606 (`send_to_telegram()`)
**Description:**
The Telegram bot token is passed as a subprocess argument to `curl`, making it visible in the process command line. On any Unix system, any user can view all running processes' command lines via `ps aux` or `/proc/*/cmdline`.
**Proof of Concept:**
```bash
# While the script is running (or just after it finishes):
ps aux | grep curl
# Output reveals token:
# curl -s https://api.telegram.org/bot123456789:ABCdefGHI/sendMessage -d chat_id=... -d text=...
```
**Impact:**
- Any user on a shared system can steal the bot token
- Attacker can send arbitrary messages from the bot
- Attacker can use the bot for spam, phishing, or social engineering
- If the bot is in group chats, attacker can extract member information
**Fix:**
Use curl with `--oauth2-bearer` or environment variable approach. The bot token should NEVER appear in the command line. Recommended fix:
```python
import os
def send_to_telegram(...):
bot_token = os.environ.get("BOT_TOKEN")
chat_id = os.environ.get("CHAT_ID")
if not bot_token or not chat_id:
return
env = os.environ.copy()
# Use a temp file or proper curl auth method
# Actually Telegram bot tokens cannot be passed via header
# Instead: use Python's requests library which handles this securely
# OR: write token to a file with 0600 perms and use curl's --config option
```
**Proper fix using Python `requests` library:**
```python
import requests
def send_to_telegram(...):
bot_token = os.environ.get("BOT_TOKEN")
chat_id = os.environ.get("CHAT_ID")
if not bot_token or not chat_id:
return
url = f"https://api.telegram.org/bot{bot_token}/sendMessage"
payload = {"chat_id": chat_id, "text": text, "parse_mode": "HTML"}
# Token stays in memory, never in command line
resp = requests.post(url, data=payload, timeout=10)
```
**Interim mitigation:** If you must use curl, write the token to a temporary file with mode 0600 and use curl's `--config` flag, then delete the file immediately after.
---
### 6.3 HIGH: HTML Injection in Telegram Messages (XSS-adjacent)
**Location:** Lines 614-661 (`send_to_telegram()`)
**Description:**
Event titles and other data from the Polymarket API are inserted directly into Telegram messages with `parse_mode=HTML`. No HTML escaping is performed. Characters like `<`, `>`, `&` are not escaped.
**Attack scenario:**
1. Polymarket (or an attacker who compromises Polymarket data) includes a malicious title like:
- `<script>alert('XSS')</script>`
- `<img src=x onerror=alert(1)>`
- `Team A &amp; Team B` (ampersand not escaped renders as `&amp;amp;` or breaks parsing)
2. The bot sends this to Telegram
3. Telegram renders the HTML, potentially executing JavaScript in the context of the Telegram web client
**Note:** Telegram's HTML parser is restrictive (`<b>`, `<i>`, `<a>`, etc. only), so classic XSS is limited. However:
- Malformed HTML can crash the Telegram client
- Attribute-based injections in `<a>` tags could be possible
- The `<` and `>` characters themselves are illegal in Telegram HTML and will cause parse errors
**Current vulnerable code (simplified):**
```python
title = e.get("title", "?")
lines.append(f"<b>{i}.</b> <a href=\"{url}\">{title_clean}</a>")
# title_clean is title.split(" - ")[0].strip() -- no HTML escaping
```
**Fix:**
```python
import html
def escape_html(text):
"""Escape HTML special characters for Telegram."""
return (text
.replace("&", "&amp;")
.replace("<", "&lt;")
.replace(">", "&gt;")
.replace('"', "&quot;"))
title = e.get("title", "?")
title_escaped = escape_html(title_clean)
lines.append(f"<b>{i}.</b> <a href=\"{url}\">{title_escaped}</a>")
```
---
### 6.4 MEDIUM: Insufficient Input Sanitization on `--search` Parameter
**Location:** Line 39 (`fetch_page()`)
**Description:**
The `--search` argument is only sanitized with `.replace(' ', '%20')`. This only handles spaces. Other URL-sensitive characters (`#`, `?`, `&`, `%`, `+`, etc.) are not properly URL-encoded.
**Attack scenario:**
A user passes `--search "Team%20A"` expecting to search for "Team%20A" literally, but `%20` gets decoded to a space by the URL parser, searching for "Team A" instead.
More concerning: if the search term contains `#`, it could truncate the query:
```bash
--search "Team#A" -> URL becomes ?q=Team#A&limit=50...
# Everything after # is treated as URL fragment, not part of the query
```
**Current code:**
```python
url = (f"{base}?q={q.replace(' ', '%20')}&limit={PAGE_SIZE}&page={page}"
f"&search_profiles=false&search_tags=false"
f"&keep_closed_markets=0&events_status=active&cache=false")
```
**Fix:**
Use `urllib.parse.quote()` for proper URL encoding:
```python
from urllib.parse import quote
def fetch_page(q, page=1, ...):
base = "https://gamma-api.polymarket.com/public-search"
encoded_q = quote(q, safe='')
url = (f"{base}?q={encoded_q}&limit={PAGE_SIZE}&page={page}"
...)
```
---
### 6.5 MEDIUM: No Bounds Checking on `--detail` Argument
**Location:** Lines 778-785 (`main()`)
**Description:**
The `--detail N` argument is used to index into `result["match_events"]` without proper bounds checking. While there is a fallback (`idx = 0` if out of range), the logic silently defaults to index 0, which may not be what the user intended.
```python
idx = args.detail - 1 # User provides 1-indexed
if idx < 0 or idx >= len(result["match_events"]):
idx = 0 # Silently defaults to first event
detail_event = result["match_events"][idx]
```
**Impact:**
- Low security impact, but confusing UX
- User expects event #100 but gets event #1 silently
**Fix:** Warn user if index is out of range instead of silently defaulting.
---
### 6.6 MEDIUM: Potential Denial of Service via Large API Response
**Location:** Lines 53, 79-81, 357-358, 393-403
**Description:**
The code loads the entire API response into memory via `json.loads()`. If a malicious or compromised Polymarket API returned an extremely large JSON payload (gigabytes), the script could exhaust system memory.
**Additional issue:** `fetch_all_pages()` loops up to 100 pages, each with up to 50 events. While bounded, there's no size limit on individual events or their nested `markets` arrays.
**Fix:**
```python
# Add response size limits
MAX_RESPONSE_SIZE = 10 * 1024 * 1024 # 10MB
if len(r.stdout) > MAX_RESPONSE_SIZE:
raise ValueError(f"API response too large: {len(r.stdout)} bytes")
```
---
### 6.7 LOW: Bare `except:` Clauses Swallow Security-Relevant Errors
**Location:** Lines 169, 183, 269, 308, 456
**Description:**
Bare `except: pass` catches ALL exceptions including `KeyboardInterrupt`, `SystemExit`, `MemoryError`, and `OSError`. This silently hides errors that might indicate security problems (e.g., timeouts could suggest a DoS attack in progress).
**Current code:**
```python
try:
end_dt = datetime.fromisoformat(end_str.replace('Z', '+00:00'))
...
except:
pass # Silently ignores ALL errors
```
**Fix:**
```python
try:
end_dt = datetime.fromisoformat(end_str.replace('Z', '+00:00'))
...
except (ValueError, TypeError):
pass # Only catch expected exceptions
```
---
### 6.8 LOW: No Rate Limiting on API Calls (DoS vector)
**Location:** Lines 74-76 (`fetch_all_pages()`)
**Description:**
An attacker who can pass arguments to this script (e.g., via a web wrapper) could repeatedly call the Polymarket API in a loop, potentially:
1. Getting the user's IP rate-limited or banned by Polymarket
2. Consuming the user's bandwidth
3. Causing resource exhaustion on the host
**Fix:** Add a cooldown between runs if called repeatedly, or use a token bucket rate limiter.
---
### 6.9 Security Summary Table
| # | Issue | Severity | Exploitable Remotely | Fix Complexity |
|---|-------|----------|----------------------|----------------|
| 1 | Telegram bot token in process cmdline | CRITICAL | No (local access needed) | Easy |
| 2 | HTML injection in Telegram messages | HIGH | No (needs Polymarket compromise) | Easy |
| 3 | Insufficient `--search` sanitization | MEDIUM | Yes (any user input) | Easy |
| 4 | No bounds check on `--detail` | MEDIUM | Yes (any user input) | Trivial |
| 5 | Large API response can exhaust memory | MEDIUM | Yes (API or MITM) | Easy |
| 6 | Bare `except:` swallows errors | LOW | No | Trivial |
| 7 | No API rate limiting | LOW | Yes (with script access) | Medium |
---
### 6.10 Recommended Immediate Actions
**If you are currently using this skill with a Telegram bot:**
1. **ROTATE YOUR BOT TOKEN IMMEDIATELY** — Go to @BotFather and use `/revoke` to invalidate the current token. Generate a new one.
2. **Do not use the `--telegram` flag** on shared/multi-user systems until Issue #1 is fixed.
3. **Do not use `--search` with untrusted input** until Issue #3 is fixed.
4. Monitor your Telegram bot's `getUpdates` for unauthorized access.
**Safe usage until fixes are deployed:**
```bash
# Without Telegram (safe):
polymarket-browse --category "Counter Strike" --limit 5
# With Telegram (UNSAFE on shared systems until fix is deployed):
# NOT RECOMMENDED until security issues are addressed
```
---
### 6.11 Fix Priority Order
1. **FIRST (CRITICAL):** Fix Telegram bot token exposure — use Python `requests` library instead of curl subprocess, OR use curl with `--config` file approach
2. **SECOND (HIGH):** Add HTML escaping for Telegram messages
3. **THIRD (MEDIUM):** Fix `--search` URL encoding with `urllib.parse.quote()`
4. **FOURTH (MEDIUM):** Add `--detail` bounds checking
5. **FIFTH (MEDIUM):** Add response size limits
6. **SIXTH (LOW):** Replace bare `except:` with specific exceptions
---
### Appendix A: Longest Lines in browse.py (for targeted refactoring)
| Line | Chars | Location | Content Summary | | Line | Chars | Location | Content Summary |
|------|-------|----------|-----------------| |------|-------|----------|-----------------|