Improved email contact form

Bug #832659 reported by Kitserve
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Pluck CMS
Fix Committed
Medium
Unassigned

Bug Description

Attached modified version of contactform.site.php from Contact Form module. It includes the following improvements:
Checks for SMTP header injection in $name and $header.
Set Return-Path and add some useful SMTP headers, e.g. Hotmail style X-Originating-IP.
Makes "message sent" or "sending failed" messages more easily visible and more grammatically correct (hardcoded English, these would need to be incorporated into the translation files).
Fixed message subject (missing space before $name).
Changes $message to plain text instead of html, removing some security risks.
Removed sanitize calls, as these resulted in ugly escaped quotes and didn't provide extra security.

I'm using this on my site and it works nicely. It may have problems on servers where $_SERVER['SERVER_ADMIN'] is not set though.

Tags: contact-form
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Sander (sanderth)
tags: added: contact-form
Revision history for this message
Sander (sanderth) wrote :

Thanks Kitserve for your code. In rev. 394 I updated the contact form, based on your code. I improved it a little bit further by adding UTF-8 encoding to the sent message.

If someone has time to do some more testing on it (especially concerning compatibility with mail applications / web mail services), that would be great. I can confirm that messages sent with current code work with both Squirellmail and Horde webmail applications.

Changed in pluck-cms:
importance: Undecided → Medium
milestone: none → 4.7.1
status: New → Fix Committed
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :

Looks good, I can confirm that the new version works correctly with Roundcube (0.5.4). The Base64 encoding on the From: field triggers FROM_EXCESS_BASE64 in SpamAssassin, but I'm not sure there's a straightforward fix to that. Hopefully it won't turn out to be a big problem.

I've attached a modified version of data/inc/lang/en.php that corrects the contactform messages (e.g. "send" -> "sent"), as well as a couple of other minor grammatical fixes and code tidying.

Presumably custom modules can include their own language code rather than modifying the central lang/*.php files, is there a particular reason this hasn't been done for core modules?

Revision history for this message
Sander (sanderth) wrote :

Ok, thanks for the corrected language file, I will commit it soon.

About the language files for modules: there is a simple reason to keep the strings for the core modules in the main language file: logistically, it's much more work to maintain separate lang files for each module, and also because our translation software doesn't support it. For the core modules, it's just a lot of work and it doesn't have any real advantages.

Revision history for this message
Kitserve (ubuntu-kitserve) wrote :

Apologies for re-opening an old bug, but I have a minor feature request: it would be great if the message subject was set to something more specific than 'Message from your website from X'. I run a number of Pluck sites at the moment, and it's often not easy to tell which site a message came from.

If I get a chance in the next couple of weeks I will try to upload a modified lang file that does this, along with a couple of other tweaks that have been on my mind.

Revision history for this message
Kitserve (ubuntu-kitserve) wrote :

Okay, it took months rather than weeks to get round to it, but I finally made the improvements to the contact form. Attached are an updated version of contactform.site.php and some modified language files.

Key changes from last version:
Language file contains %s placeholders in email subject string $lang['contactform']['email_title'].
Contact form code replaces %s placeholders with website name and sender name.
Contact form code also detects whether sender name or site title include non UTF-7 characters, and if so base64 encodes them.

N.B. if the sender name only contains UTF-7 characters, no encoding takes place, which avoids triggering FROM_EXCESS_BASE64 in SpamAssassin. Also N.B. that the encoding would be a bit neater if we used quoted-printable encoding, but this would require PHP function imap_8bit(), which requires the IMAP module. I wanted to avoid requiring any extra dependencies.

Unfortunately my knowledge of languages other than English is rather limited. I've attached modified language files for English, French, German, Italian and Spanish, but $lang['contactform']['email_title'] needs to be edited for the other languages to include the necessary %s placeholders.

Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :
Revision history for this message
Kitserve (ubuntu-kitserve) wrote :

Apologies to all for re-opening this ticket. Attached is a newer version of the main contactform file. It includes two major changes:
1. Validate sender's email address as far as possible.
2. Change address on From: header, to avoid problems due to e.g. Yahoo's new DMARC policy.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.