Prevent XSS attacks: wrap echo calls in safe_echo()

[cwebber]

We should write a safe_echo() which is more or less like the following:

function safe_echo($input) {
    echo(htmlspecialchars($input));
}

All calls to echo() that aren't known to be safe (and don't need to output characters like "<" unescaped) should be called with this. (Likewise, all calls to print and print_r I think also go to the browser?)

This looks to be nearly 6000 calls:

$ grep -nH -e "echo" ~/devel/lisc-ttm/ -iR --exclude-dir=".vagrant" --exclude-dir=".git" | grep php | grep -v htmlspecialchars | wc -l
5940

Luckily, I think @kfogel is right that probably this can be mostly-automated in emacs, though probably all replacements need to be screened for the rare cases where they do need to print out characters unescaped (what might those be though?)

[tsyesika]

You could also use PHP's strip_tags which will actually remove any HTML tags, not sure if that's something you would find useful. I imagine we're not wanting the user to be submitting HTML to us.

[kfogel]

Offer of division of labor:

Christopher or Jessica, if you will write & test the safe_echo() function and make it available to me in some reasonable include file, I'll take care of updating the entire codebase to use it.

[tsyesika]

I have added this function. I discussed this with Chris in IRC and he seemed to think strip_tags was a bad idea since it's possible users will enter data which could look like HTML so simply escaping it is sufficient.

See commit: https://github.com/OpenTechStrategies/lisc-ttm/commit/f7134449d949cfb07471a80c26b6bbf5cf4b29cf

[cecilia-donnelly]

Just noting that there are some occasions where an echo does include (desired) html characters. It may be rarer than I initially thought, though, because I wasn't able to quickly find an example. If I recall correctly, it usually happens when a "response" from a POST is displayed to the user. This could be search results or in a report.

[kfogel]

By the way, I am treating this as something that can be done after the open-sourcing. There is theoretical possibility of XSS attack here, but I think turning it into a real attack wouldn't be so easy. If I'm being too complacent, though, let me know -- I can bump up the priority if needed.