Using AI to Secure Legacy Software

In 2006, I made the decision to stop writing software for Windows (as much as I did at the time), and move to building web applications. The idea was that, unlike desktop applications where I don’t see my users and can’t do updates easily, in a web app I can.

I spoke with my brother, who recently started doing some web work himself and asked what he’d recommend, and he said a lot of sites are using PHP. Wikipedia, Facebook, and other major sites of the era were great examples of some of those sites. I knew very little about web design and development beyond HTML, and for sure didn’t know a darn thing about security.

My first application was a forum, and I’m going to share some of the code here. I absolutely don’t recommend using it.

The Code

 <?php
 include("header.php");

//Pull viewing user.
	include('getuser.php'); 
	$xx = $userret;

$ip=@$REMOTE_ADDR;
$file = fopen("ipmain.txt" , "a");
fwrite($file, $ip . "\r\n");
fwrite($file, $xx . "\r\n");
fwrite($file, "comment\r\n");
fclose($file);

$orgbb = $_POST["combod"];
//Strip linefeeds, replace with BBCode [br]
$orgbb = nl2br($orgbb);
$orgbb = str_ireplace("<br />", "[br]",$orgbb);
$orgbb = str_replace("\r", "", $orgbb);
$orgbb = str_replace("\n", "", $orgbb);
$Vie = $_POST["a"];
$file = fopen("article/comments/" . $Vie . ".txt", 'a');
fwrite($file, date(DATE_RFC822) . "\r\n");
fwrite($file, $xx . "\r\n");
fwrite($file, $orgbb . "\r\n");
fclose($file);

echo "<td valign='top' bgcolor='#ffffff' class='mainblur'>
<center>
<font size='5' face='arial' color=e1e100>Comment Added</font><br />
<font size='1' face='arial' color='#e1e100'>You can return Home to see the new post!</font><br/><br /> <a href='view.php?v=" . $Vie . "'><font size='2' face='arial' color='#e1e100'>Click here to return to the article!</font></a>";

//sleep(5);
$redir = "Location: http://site.com/view.php?v=" . $Vie;
echo header($redir);
?>
</form>

Going to go ahead and stop you here: Yes, this code is abhorrent. Deprecated HTML tags, inline styling, presentation and logic are mixed, don’t learn from this code. I was still learning, it worked, and it was bad. I’m only focusing on security.

Crazy. If OWASP Top 10 was a list with examples, this is a pretty compelling example of most of them.

To summarize what this script does:

• Outputs an HTML header (logo, links etc)
• Pulls in a user management function to get the username ($userret is “User Returned”, which I of course copied to $xx for readability)
• Then, it logs the user’s username and IP to a file
• Next, pull in “combod” (comment body). Because I used line endings to separate fields and records (I was afraid to learn SQL back then), I had to convert the line endings to BBCode ([br]) and remove them.
• The comment, username, and date are logged to a file (the article ID)
• A message is posted notifying the user that the comment was posted, giving them a link to click to get to the article
• A redirect is sent to send the user where I wanted them

Functional and Security Problems

Without using AI, I’m going to review my own old code and call out vulnerabilities and functional problems.

• Right off the bat, I’m pulling an IP address using a PHP Magic Constant.
  • If you were to put this behind a reverse proxy, it would miss the client’s IP since it would be in HTTP_X_FORWARDED_FOR instead
  • This leaves it potentially injectable, and we log it to a file without validating the address is the expected format.
• Next, I’m pulling in the POST variable “a” into the variable “$Vie”. Notably, I immediately lob this into a file path
  • This can be hit with a path traversal (using ../../ in the path) to move up folders and potentially replace files at the operating system level. Imagine if they replaced “../../../../WINDOWS/SOL.EXE” with something dangerous! (Yes, I also hosted this on Windows 2000 Server).
  • You may think that the way we concatenate the string, it has to end in “.txt”, but computers end strings with a CHR(0) (null byte). By adding a null byte to the end of the “a” parameter, I could potentially drop the .txt off the end.
• Next, I store “$orgbb” to the file. The only validation here is that I clean up line endings. I don’t make sure the message isn’t blank, spammy, repeating a previous one (double posting), contains anything insecure. So on.
• Later, I inline “$Vie” into the HTML. In theory, you could inject this with XSS (cross site scripting) payloads. That said, I actually consider this a lower vulnerability since most anything that would work for XSS would also likely end in a fatal error earlier in the script when it was used to open a file.
• Finally, I’m using $Vie to feed into a redirect. I also think this is less of a vulnerability since it is likely the script failed here. You cannot send HTTP headers after sending body content, so this would have also been a fatal error. I likely didn’t catch it back in the day as it was the expected end of the page.

What was that you said about AI?

I wanted to compare my evaluation of the code to what AI will return, so now I’ll go ahead and paste the results from ChatGPT’s 4o model:

My Prompt

I have a script I wrote back around 2006 that allowed a guest to post a comment. It has several security vulnerabilities in it, as well as a host of deprecated ways of building web apps. What security problems do you see? (code followed here)

The Response

Your script has several security vulnerabilities and outdated practices:

1. Lack of Input Sanitization/Validation: The $_POST data is directly used without sanitization or validation, making it vulnerable to SQL injection, XSS (cross-site scripting), and code injection attacks.
2. Deprecated $REMOTE_ADDR Usage: @$REMOTE_ADDR is outdated. Use $_SERVER['REMOTE_ADDR'] for a user’s IP address, though be cautious of proxies or spoofing.
3. File Manipulation Vulnerabilities: Writing directly to files (fopen, fwrite) without validating or sanitizing filenames makes it vulnerable to directory traversal and arbitrary file write exploits.
4. Open Redirect: The header("Location: ...") can lead to open redirect vulnerabilities if not properly validated.
5. Outdated Functions: include() can lead to file inclusion vulnerabilities if the files being included are not carefully controlled. Also, using nl2br() and str_ireplace() without input sanitation allows potential for XSS.

Recommendations:

• Sanitize/escape all user inputs (e.g., using htmlspecialchars(), filter_var()).
• Use parameterized queries for database interaction (if you introduce one).
• Replace fopen() with proper database usage for storage.
• Validate and sanitize filenames and paths.
• Secure your redirect with checks for trusted domains.

Closing Thoughts

AI Did pretty good here, though some commentary I have is that “include()” is not outdated. I have changed in modern code to use “require” or “require_once”, as optionally including code seems janky, but the PHP site does not indicate it is out of date.

Similarly, str_ireplace(), which does a case insensitive string replacement, is also not outdated and is really useful. It’s note about XSS, however, is accurate.

The recommendation to use htmlspecialchars() is good for output (like the link output in the dialog box), but I generally wouldn’t store it this way.

The recommendation to replace file storage with database storage is good too.

A few years later, in 2008/2009, I wrote a new forum package that rolled in this and a ton of other great protections. I may share that work too!

By the way, I asked AI to generate my cover photo for this blog, and here’s the response: