Fix multiple security issues in legacy endpoints

[hasankhan]

Fixes several security issues found via a code review of the legacy non-Yii endpoints and a few Yii components.

1. SQL injection in public/search_redirect.php (Critical)

X-Forwarded-For / HTTP_CLIENT_IP flowed unescaped into an INSERT on the search DB, and mysqli_error was echoed back on failure (error-based exfiltration). Replaced with a mysqli_prepare + bind_param call and removed the error echo.

2. Reflected XSS in public/share.php (High)

\['link'] was echoed unescaped inside href="..." attributes for the Facebook / WhatsApp / Telegram share buttons. An attacker-controlled form post could break out of the attribute and inject event handlers. Now HTML-escaped with ENT_QUOTES and constrained to a site-relative path via regex.

3. Unauthenticated cache flush in IndexController::actionFlushCache (High)

The route GET /yiiadmin/flushcache was publicly reachable with no auth, allowing any visitor to repeatedly flush Memcached (cache-stampede DoS on the origin DB) or delete arbitrary cache keys. Now requires POST and a loopback caller (127.0.0.1 / ::1), matching how server-local maintenance scripts would invoke it.

4. CRLF / header injection in ElasticConnection (Medium)

User-Agent and the forwarded client IP (taken from request headers) were concatenated into outbound HTTP headers joined with \r\n. A crafted User-Agent could inject additional headers or smuggle a second request into the Elasticsearch connection. Added a sanitizeHeaderValue helper that strips \x00-\x1F and \x7F.

5. Mail header injection in public/processer.php (Medium)

\['email'] was placed into the Reply-To header with no validation, enabling Bcc: / Content-Type: injection and turning the form into an open relay using the SES credentials. Now validated with FILTER_VALIDATE_EMAIL and rejected if it contains CR/LF, with fallback to adminEmail.

All five files pass php -l.

[hasankhan]

Closing — no further changes will be made to this fork.

[hasankhan]

Adversarial review — these are good fixes overall, but several have gaps that mean the fix is incomplete or the endpoint is broken. Numbered to match the PR description.

3. actionFlushCache — likely broken by this change

Two blockers:

1. Yii CSRF will reject every call. IndexController only disables CSRF for ajaxhadithcount (beforeAction at line ~41); flush-cache is in the cache filter's except list but not exempted from CSRF. With the new getIsPost() requirement, a loopback maintenance script doing curl -X POST http://localhost/yiiadmin/flushcache has no CSRF token and will get a 400. The endpoint is effectively unreachable after this PR. Add to beforeAction:

   if ($action->id == 'flush-cache') { $this->enableCsrfValidation = false; }

   …or keep it GET (the action is idempotent; the IP allow-list is the real defense).

2. getUserIP() is REMOTE_ADDR, which is not loopback behind a reverse proxy. If Yii is fronted by nginx/Apache on the same host and PHP-FPM gets the real client IP forwarded (e.g. set_real_ip_from / RemoteIPHeader), REMOTE_ADDR will be the public client IP and the loopback check fails for legitimate local callers. Conversely, if Yii's trustedHosts is ever configured to trust X-Forwarded-For, an attacker can spoof 127.0.0.1. Worth either (a) reading $_SERVER['REMOTE_ADDR'] directly and documenting the deployment assumption, or (b) gating on a shared secret header instead of IP. As written this is fragile.

Please also verify manually post-merge that curl -X POST http://127.0.0.1/yiiadmin/flushcache actually returns 200 — I'm fairly sure it currently won't.

2. share.php — regex allows things you probably didn't intend

• The character class includes : and ?, so /foo:bar?x=1 is accepted. For Facebook (?u=https://sunnah.com<link>) any ? or & in $link collapses into the outer u= query string and corrupts the share URL. Consider restricting to ^/[A-Za-z0-9_\-./]*$ and stripping query/fragment, since real hadith permalinks never need them.
• strpos($rawLink, '//') !== 0 only catches a leading double slash. The regex itself permits // anywhere, but since the value is appended after https://sunnah.com, the only dangerous case is //evil.com, which is caught — so this is fine, just worth a comment noting why.
• nl2br(htmlspecialchars($hadithText)) is rendered into the page but $hadithText is otherwise unconstrained — fine for the preview, but note hadithText is also rawurlencoded into https://twitter.com/intent/tweet?text=… with no length cap. Twitter will truncate; not a security issue, flagging for awareness.

4. ElasticConnection::sanitizeHeaderValue — fix is correct but incomplete

• No length cap. A 1 MB User-Agent will be cleaned and then sent verbatim to ES on every request. Suggest mb_substr($clean, 0, 512) or similar.
• getClientIp() (unchanged) trusts CF-Connecting-IP and X-Forwarded-For from the request unconditionally — if the app is ever exposed without Cloudflare in front (staging, direct origin hit), any client can set these. Out of scope for this PR but worth a follow-up issue; sanitization here mitigates the smuggling vector but not the spoofing one.

5. processer.php — only email is validated

Reply-To is fixed, but $_POST['type'], $_POST['othererror'], $_POST['re_additional'], and $_POST['urn'] flow into $subject and $fullString. $subject = "[Error Report] URN ".$urn; — $urn from POST goes straight into the Subject header. PEAR Mail will happily pass a CRLF in the subject to the SMTP DATA and inject headers. Please also validate $urn (e.g. ctype_digit) and strip CR/LF from $subject defensively. Otherwise this fix moves the injection point one field over.

1. search_redirect.php — two leftover issues

• Line 32 (unchanged): header('Location: /search/'.addslashes(url_encode(trim($_GET['query'])))); — addslashes on a Location header is meaningless for HTTP and does nothing for CRLF. url_encode() (the local function) does rawurlencode at the end which does encode \r\n, so it happens to be safe today, but the addslashes is misleading dead code and url_encode is doing the actual escaping. Worth removing the addslashes.
• IP value (HTTP_CLIENT_IP / HTTP_X_FORWARDED_FOR) is fully attacker-controlled and now bound as a string. Not SQLi anymore, but the column may overflow / store junk. Consider filter_var($IP, FILTER_VALIDATE_IP) with a fallback to REMOTE_ADDR.
• Silently swallowing connect/prepare failures (no logging) makes the original or die(mysqli_error($con)) behavior — broken as it was — strictly less debuggable. Consider error_log() on failure.

General

• Per repo conventions there are no working tests, but at minimum each of these would benefit from a manual repro recipe in the PR description (curl one-liner per fix) so a reviewer / future-you can re-verify. I'd specifically ask for one for flushcache because I think it's broken.
• No changes to public/report.php — is it intentional that it was reviewed and found clean, or just not yet looked at? Worth a note.

Happy to re-review once the flush-cache CSRF/POST issue and the subject-header injection in processer.php are addressed.