Mail Header Injection Vulnerability and other things

[Sepodati]

First of all, great program. I just had to make some changes to it for a client and it only took a couple hours because your code was pretty organized and it was easy to find the areas I needed to modify.

That being said, I came across a few issues that may or may not be already fixed.

1. Mail Header Injection in "email_listing.php"

You have the following line of code:

$header = "From: ".$sender." <".$sender_email.">\r\n";

that takes input directly from the user. A malicious user could send a $sender or $sender_email value with newlines in it and add headers to the email being sent out. This means they could change the From, Reply-To, etc headers and the content of the message itself (hiding what you add later).

2. My client didn't want to ask for most of the default information from users who just sign up to save listings and searches. I saw in the admin area where you could delete the extra fields, but it didn't affect the member registration. I changed the table in "member_signup.php" from "userformelements" to "memberformelements" and then it seemed to work correctly. Is this a typo for the table name or am I doing the wrong thing?

3. Error reporting

I have my error reporting set to E_ALL in PHP and when I tried to install the program, I got a bunch of warnings about undefined variables and what not. I simply added the line

error_reporting(E_ALL ^ E_NOTICE ^ E_WARNING);

to "common.php" to hide the NOTICE and WARNING messages. You may want to add this to your own code so people do not have to do it themselves.

4. Cross Site Scripting

I'm thinking you may have multiple instances of cross site scripting vulnerabilities. Just looking at the file I have open, I see this line

echo "$lang[email_listing_sent] $to.<P><a href=\"listingview.php?listingID=$listingID\">Return to listing</a>   ";

where both $to and $listingID are taken directly from user input and displayed back to the user. Either one could be HTML code from the user and be used to rewrite/hijack the entire page.

This is just a few things I noticed in the couple hours I worked on modifying the program. Hopefully they've already been fixed or at least you'll look into it. Thanks.

---John Holmes...

[joe schmoe]

1.Yeah, I brought this up a while ago. I posted my "throw a blanket over it" fix in this thread from last summer:
http://support.open-realty.org/showt...il_listing.php
As it mention in the thread, it's certainly far from airtight, but it's better than nothing.


4. I'm not sure what the vulnerability is here? I mean, the most someone could do is direct to the script from another site to make it read:
Email sent to:this site sucks
or something like that. It doesn't really compromise site security in any way... or am I missing something here?

[Sepodati]

1. Yeah, that's not much of a fix (the HTTP_REFERER one). HTTP_REFERRER is easily spoofed and not all browsers send it, anyhow. I would not use a script that relied upon it.

Preventing Mail Header Injection is rather simply. You just need to remove any line breaks from anything that you put in the header. If you want to make the email "From:" a user supplied name and email, then just validate both of them so there are no line breaks. If there are no line breaks (either remove them or fail validation, depending upon the script), then there can't be any mail header injection.

4. Maybe page that was a poor example, but the bottom line is that if you're outputting unvalidated/unsantized user data to your page, your letter the user write the page for you. This includes HTML, DHTML, JavaScript, etc. The can make a <div> that covers all of your content and displays their own. What if they use it to make a false listing with false contact information? It just depends on the site for how much damage this can do. Maybe it'll just make a bad name for the site, maybe more. These kind of Cross Site Scripting attacks vary in the damage they can do and the method to introduce them to users. But the bottom line is that they are simple to prevent and even if the risk is low, they should be fixed.

The fix is to validate any user input. If you're asking for an email, make sure it's an email. If you're asking for a number, make sure it's a number. If you're asking for text that you want to display again, run it through htmlentities() first so any HTML, JavaScript, etc are displayed, rather than rendered to the user...

---John Holmes...

[joe schmoe]

1. IE, Opera, Safari, and all Mozilla based browsers send HTTP_REFERRER. So, at most you're only ostracizing MUCH less than 1% of viewers.

But you're certainly right about the spoofing. That's why this is not a great fix. As I mention in the other thread, I think the best way to do this would be using PHP sessions to determine if the form were being accessed by someone visiting your site. Unfortunately, I just don't have the PHP-coding talent to implement such a solution :( . Do you have any ideas on how this might be accomplished?

4. [snip]The can make a <div> that covers all of your content and displays their own. What if they use it to make a false listing with false contact information? It just depends on the site for how much damage this can do. Maybe it'll just make a bad name for the site, maybe more.

4. Ah, I had not thought about the potential to make a fake listing. You're right it could certainly be done, but the "attacker" would have to direct to the page from their own site or an email link, putting full legal responsibility upon them. And besides, I think the legal disclaimer would "cover your ass" in this circumstance.

As you mention though, this could certainly ruin a site's reputation.

[Sepodati]

putting full legal responsibility upon them. And besides, I think the legal disclaimer would "cover your ass" in this circumstance.

You'd be surprised what may or may not hold up in court. For something that's trivial to fix, it's not worth taking the chance.

---John Holmes...