Fix SQLi and XSS in Legacy PHP Without Changing Output

This is for anyone who has to fix SQL injection and XSS in an old PHP backend that mobile apps still call, where changing even one byte of a response might break an app version you cannot update. Our conclusion: escape inline at the exact lines that build SQL and output, keep query(), and prove the output did not change by diffing saved real responses, not by reading the patch. That found two holes the source review missed.

The backend in question runs on shared hosting (Hostinger, LiteSpeed with the host's CDN in front). It has PHP JSON APIs used by eight mobile apps, plus PHP pages from the WebView era. Old installed app versions still open those pages inside a WebView, so they cannot simply be deleted. All numbers below are from the audit on 2026-11-09.

What we found

The seven unused folders were the scariest part. They included a REST-style login endpoint that compared plaintext passwords and test scripts that could insert and delete rows. Nothing referenced them, but they were reachable from the internet. We moved them outside the web root rather than deleting them. They now return 404, and if something turns out to depend on them, moving them back takes seconds.

The scanner, and the two things it missed

We started with a heuristic scanner, a short Python script with regular expressions:

• HIGH: $_GET, $_POST or $_REQUEST appears directly on a line that contains SQL.
• MED: a variable that was assigned from input, without a sanitizer, appears in a SQL string later.

"Contains SQL" meant a keyword list: SELECT, INSERT INTO, UPDATE … SET, DELETE FROM, WHERE, ORDER BY, LIMIT, VALUES, LIKE, or a ->query( call. It found most cases. Two classes slipped through.

Miss 1: continuation lines. Old code builds queries line by line:

$Query  = "SELECT … FROM … WHERE col1 = '".$_GET['k1']."'";
$Query .= " AND col2 = '".$_GET['k2']."'";

The first line was flagged. The second was not, because AND was not in the keyword list. The scanner saw an assignment from input, marked $Query as tainted, and moved on. It did not report the line itself.

Miss 2: taint through arrays. This pattern appeared in a search page:

$arr[0] = $_GET['keyword1'];
$arr[1] = $_GET['keyword2'];
sort($arr);
$kw = implode(' ', $arr);
…
$Query .= " AND name LIKE '%".$kw."%'";
$Query .= " OR  code = '".$kw."'";

The scanner tracked plain variables ($x = …), not array elements ($arr[0] = …). The taint went into the array, was sorted, joined into a string and then used in both LIKE '%…%' and = '…'. The scanner never saw the link.

After we knew these two shapes, manual tracing found three more files with the array pattern. It also found something worse: a shared util class copied $_GET into a global variable, and a different file used that global in its SQL. No per-file scanner sees that. You only find it by following the include chain.

The lesson is simple: a regex scanner gives you a list to start from, not proof that the code is clean.

The fix: inline escaping, not prepared statements

Prepared statements are the right answer for new code. Here they would have changed the API output. With mysqlnd, prepared statements return integer and float columns as native numbers, so a JSON field that was "7" becomes 7. Old app builds that expect a string can crash on that. The measurements are in our post on prepared statements and JSON types.

So we escaped each value only on the lines that build $Query or $Sub_Query, and touched nothing else:

// before
$Query .= " AND col2 = '".$_GET['k2']."'";

// after
$Query .= " AND col2 = '".$Conn->real_escape_string((string)($_GET['k2'] ?? ''))."'";

The (string) cast handles k2[]=x array input, which would otherwise raise a TypeError on PHP 8. The ?? '' keeps a missing parameter as the empty string that the old code effectively used.

Why we could expect byte-identical output for real traffic: real_escape_string only touches seven characters: NUL, \n, \r, \, ', " and Ctrl-Z. Every byte of a UTF-8 multibyte character is 0x80 or higher, so Korean, Japanese and digit inputs (which is everything the apps actually send) pass through unchanged. The SQL string is the same, so the result set and the JSON are the same.

One caveat for anyone working with legacy Japanese or Chinese systems: this reasoning only holds when the connection charset is UTF-8, set with set_charset('utf8mb4'). With Shift_JIS or GBK, the second byte of a multibyte character can be 0x5C (the backslash), and escaping with the wrong charset is a known way to break out of the quotes.

Final counts:

XSS: the context decides the escape

The pages echoed request parameters back into HTML. The fix depends on where the value lands:

The last row needs an explanation:

<a onclick="location.href='?d=<?= $d ?>'">          // vulnerable
<a onclick="location.href='?d=<?= htmlspecialchars($d, ENT_QUOTES, 'UTF-8') ?>'">  // still vulnerable
<a onclick="location.href='?d=<?= htmlspecialchars(rawurlencode($d), ENT_QUOTES, 'UTF-8') ?>'">  // ok

htmlspecialchars turns ' into '. But the browser decodes entities in the attribute value before handing it to the JavaScript engine, so the JS sees a real ' and the string ends there. rawurlencode turns the quote into %27, which means nothing to either parser. The htmlspecialchars on top changes nothing for its output, but it keeps the rule "every attribute gets escaped" simple. The real values in this spot were 8-digit dates like 20251228, which rawurlencode leaves untouched, so the output stayed identical.

Proving the output did not change

Before patching, we saved the responses of 27 pages called with real parameters taken from the apps. Every request carried a unique cache-busting query parameter so the CDN could not answer from cache (we had been fooled by cached responses before).

After deploying:

• 24 pages: byte-identical.
• 3 pages differed. They show random number predictions, so each request is different anyway.

For the random pages we did not accept "it's probably the random part". We fetched each page twice in a row and recorded which line numbers differed between the two fresh fetches. Then we compared baseline against patched:

# condensed from the comparison script
def diff_lines(a: str, b: str) -> set[int]:
    return {i for i, (x, y) in enumerate(zip(a.splitlines(), b.splitlines())) if x != y}

noise   = diff_lines(fresh1, fresh2)       # random rows only
changed = diff_lines(baseline, patched)
assert len(fresh1.splitlines()) == len(patched.splitlines())
assert changed == noise                    # 200 lines each, same set

Both sets were the same 200 lines, so only the random rows had changed. The second pass, covering the four shared includes, re-checked 8 more pages: all identical. The API snapshot comparison had 0 failures, and the site-wide URL check passed 53/53.

Testing the attack when a WAF is in the way

The obvious test, ' OR '1'='1, was useless. The host's WAF answered it with "Forbidden" before PHP ran, for the patched and the unpatched code. A WAF can hide a bug. It does not fix it, and a slightly different payload may get through.

So we used inputs the WAF ignores:

• SQL: a lone single quote. The patched endpoints returned [] or an empty result with status 200. An unescaped quote produces a SQL syntax error instead.
• XSS: a harmless probe tag, <x1>, in every parameter. Then we searched the HTML response for the literal <x1>.

The first XSS pass still reflected the probe in two places. One was a search box in a shared navigation include, the other a page title set inside the util class. Our grep of the source had missed both, because the value came in through an include and was printed far from where it was read. Searching the response found them. That is why we did a second pass over the four includes.

Failures that were already there

A regression check is only useful if you know what was broken before you started. We wrote these down first:

• One page had a PHP parse error (an unclosed brace) and had always returned 500. We did not deploy a patched version of it. Fixing it would have changed its behaviour, which was outside the scope of this work.
• Lookups that matched no rows returned 500 before and after the patch. Without a baseline, that looks exactly like a regression.

Deploying without making things worse

• Update-only mode: the deploy script aborted if a file did not already exist on the server, so a typo in a path could not create a new reachable file.
• Backup first: the server's original was downloaded before each overwrite.
• php -l on every file before upload. Local PHP 8.4 printed Case statements followed by a semicolon are deprecated for some old files. That was pre-existing and harmless, but you need to know it is old so that nobody stops the deploy over it.

When to choose which

The key point: decide what "unchanged" means before you patch, save the evidence, and let the diff tell you whether you were right.