[PHP-dev 1201] Re: mb_send_mail の引数のサニタイズについて

[Seiji Masugata]

こんにちわ、桝形です。

> > 　mb_send_mail() の引数に関してなんですが、mail() では行われて
> > いる改行コードに対するサニタイズが行われていないようで、スクリ
> > プト側でサニタイズをしていないスクリプトがスパムメール送信の踏
> > み台にされていることが確認できました。
> 
> 　この件ですが、
> 
>     ・Kent Web「PostMail」におけるメール第三者中継の脆弱性
>       http://www.ipa.go.jp/security/vuln/documents/2005/JVN_25106961_PostMail.html
> 
> と同様の問題を持っています。

この件ですが、

http://jp.php.net/manual/ja/ref.mbstring.php#mbstring.overload

関数のオーバーロードを機能使う事で、mail関数はmb_send_mail関数に
置き換わります。

この為、mb_send_mail関数でもmail関数と同様なチェックは必要だと
思います。mail関数から移植してみました。
---------------------------------------------------------------------
--- php4-STABLE-200511180155,orig/ext/mbstring/mbstring.c	2005-11-05 10:45:05.000000000 +0900
+++ php4-STABLE-200511180155/ext/mbstring/mbstring.c	2005-11-18 14:14:18.000000000 +0900
@@ -3460,6 +3460,22 @@
  *  Sends an email message with MIME scheme
  */
 #if HAVE_SENDMAIL
+#define SKIP_LONG_HEADER_SEP_MBSTRING(str, pos)										\
+	if (str[pos] == '\r' && str[pos + 1] == '\n' && (str[pos + 2] == ' ' || str[pos + 2] == '\t')) {	\
+		pos += 3;											\
+		while (str[pos] == ' ' || str[pos] == '\t') {							\
+			pos++;											\
+		}												\
+		continue;											\
+	}													\
+	else if (str[pos] == '\n' && (str[pos + 1] == ' ' || str[pos + 1] == '\t')) {	\
+		pos += 2;											\
+		while (str[pos] == ' ' || str[pos] == '\t') {							\
+			pos++;											\
+		}												\
+		continue;											\
+	}													\
+
 PHP_FUNCTION(mb_send_mail)
 {
 	int argc, n;
@@ -3475,6 +3491,8 @@
 	mbfl_memory_device device;	/* automatic allocateable buffer for additional header */
 	const mbfl_language *lang;
 	int err = 0;
+	char *to_r;
+	int to_len, i;
 
 	/* initialize */
 	mbfl_memory_device_init(&device, 0, 0);
@@ -3501,6 +3519,29 @@
 	convert_to_string_ex(argv[0]);
 	if (Z_STRVAL_PP(argv[0])) {
 		to = Z_STRVAL_PP(argv[0]);
+		to_len = Z_STRLEN_PP(argv[0]);
+		if (to_len > 0) {
+			to_r = estrndup(to, to_len);
+			for (; to_len; to_len--) {
+				if (!isspace((unsigned char) to_r[to_len - 1])) {
+					break;
+				}
+				to_r[to_len - 1] = '\0';
+			}
+			for (i = 0; to_r[i]; i++) {
+				if (iscntrl((unsigned char) to_r[i])) {
+						/* According to RFC 822, section 3.1.1 long headers may be separated into
+					 * parts using CRLF followed at least one linear-white-space character ('\t' or ' ').
+					 * To prevent these separators from being replaced with a space, we use the
+					 * SKIP_LONG_HEADER_SEP_MBSTRING to skip over them.
+					 */
+					SKIP_LONG_HEADER_SEP_MBSTRING(to_r, i);
+					to_r[i] = ' ';
+				}
+			}
+		} else {
+			to_r = to;
+		}
 	} else {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Missing To: field");
 		err = 1;
@@ -3599,12 +3640,15 @@
 		extra_cmd = php_escape_shell_cmd(extra_cmd);
 	} 
 
-	if (!err && php_mail(to, subject, message, headers, extra_cmd TSRMLS_CC)) {
+	if (!err && php_mail(to_r, subject, message, headers, extra_cmd TSRMLS_CC)) {
 		RETVAL_TRUE;
 	} else {
 		RETVAL_FALSE;
 	}
 
+	if (to_r != to) {
+		efree(to_r);
+	}
 	if (extra_cmd) {
 		efree(extra_cmd);
 	}
---------------------------------------------------------------------
Subjectについては問題無いようでした。調べ方が悪いだけかも知れません。


> 　元のメールでは、mail() ではサニタイズが行われている…と書きましたが、
> additional_headers パラメータは素通りしてしまいますので、スクリプト側
> でのサニタイズが必要になります。

additional_headers パラメータについては、自前でヘッダを作る事が要求
されているので、意見が分かれるトコロですが。。。どうしましょうね？

additional_parametersが文字列なのが良くないんでしょうかねぇ。

-- 
Seiji Masugata <s.masugata ＠ digicom.dnp.co.jp>