Create comprehensive security analysis and todo.md - found no fixes actually implemented

[Copilot]

This PR provides a thorough security analysis of the Newsgrouper codebase by comparing the comprehensive security review findings against the actual current code implementation. The analysis reveals a critical discovery: none of the security issues marked as "FIXED ✅" in the security review have actually been implemented.

Critical Findings

All major XSS vulnerabilities remain active in the codebase:

• Line 1323: URL injection vulnerability in article markup system - html "<a href='$tok_txt' target='_blank'>$tok_txt</a>" allows arbitrary HTML/JavaScript injection
• Line 1352: Dangerous subst command usage - set html [subst $out] enables code execution via malicious markup
• Lines 827, 1953: Unvalidated URL parameters in href attributes can inject javascript: URLs
• Lines 41-46: CSS color injection allows XSS via malicious color values in user preferences

Issue #19 Analysis

The TSV memory leak issue is confirmed as a real problem. The application stores Face image data indefinitely:

tsv::set Faces $addr [binary decode base64 $facedata]  # Line 1231
tsv::set Faces $addr {}                                # Line 1235  
tsv::set Faces $addr $png                             # Line 2389

This causes unbounded memory growth in long-running processes as new email addresses are encountered, with no cleanup mechanism for the binary image data.

Positive Findings

Database security is actually well-implemented - all 51+ SQL queries throughout the codebase use proper parameterized statements, providing strong protection against SQL injection.

Deliverables

• todo.md: Comprehensive 200+ line analysis document with detailed findings, code locations, impact assessments, and prioritized action items
• SECURITY_SUMMARY.md: Quick reference guide highlighting immediate critical actions needed

The security review document appears to have been aspirational rather than reflecting actual implemented fixes. This analysis provides the foundation for addressing the real security vulnerabilities present in the codebase.

Fixes #9.

---

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

[go-while]

@copilot

please review Line 1352: Dangerous subst command usage - set html [subst $out] enables code execution via malicious markup again with all context you can get and explain here in comment what the issue is and how to fix it.

[Copilot]

@copilot

please review Line 1352: Dangerous subst command usage - set html [subst $out] enables code execution via malicious markup again with all context you can get and explain here in comment w...

Line 1352 subst Command Injection Vulnerability Analysis

The Issue: Line 1352 contains set html [subst $out] where $out contains HTML template strings with unescaped user content, creating a command injection vulnerability.

Attack Vector:

1. User posts a newsgroup article containing Tcl command syntax like: *[exec rm -rf /]*
2. The markup system processes this as emphasis markup (bold)
3. At line 1352, subst executes on <strong>$html</strong> where $html contains [exec rm -rf /]
4. The subst command executes the bracketed expression as Tcl code

Data Flow:

• Line 1262: User article content flows into markup_art_line $line
• Lines 1315-1332: Content gets tokenized and processed through markup_art_tokens
• Line 1352: subst performs variable/command substitution on templates containing unsanitized user data

Fix Options:

1. Safe substitution: Replace subst with string map for simple variable replacement:

   set html [string map [list \$html $html] $out]

2. Escape user content: Sanitize $html before substitution to remove command syntax:

   set safe_html [string map {[ \\[ ] \\]} $html]
   set html [subst $out]

3. Template system: Use a proper templating approach that separates data from code

The vulnerability affects the emphasis markup system (*/_ formatting) when markup mode is enabled, allowing arbitrary Tcl command execution on the server.