[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[freewnn:01002] FreeWnn JServer Logging Option Data Corruption Vulnerability (?)
- To: freewnn@XXXX
- Subject: [freewnn:01002] FreeWnn JServer Logging Option Data Corruption Vulnerability (?)
- From: aono@XXXX
- Date: Thu, 14 Aug 2003 19:51:08 JST
- Reply-to: freewnn@XXXX
青野です。
＃先週MLに似た内容のメールを出したのですが(現時点までで)配
＃送されてこなかったので、念のため小野さんにBcc:でお送りし
＃ています。重複したらすみません。
Web検索で見つけてしまったのですが、
http://www.securityfocus.com/bid/7918 なんてことをいわれて
るようです。私の(乏しい)理解力を持ってすれば、Mandrake
Linux 9.1 のFreeWnnパッケージでは、jserverの -s オプション
に任意のファイルを指定し、wddel -nオプション(環境名なんで
すね)にそれらしい内容を書けば/etc/shadow も上書きできます
よ、ということらしいです。
…以上がおおむね先週出した内容の要旨だったのですが、その後
調べてみた結果も踏まえて書いていきます。
SecurityFocus Newsletter 第202号 (2003-6-16->2003-6-20)に
も載っているようなので、BUGTRAQ-JPに投稿された和訳
(http://www.securityfocus.com/archive/79/327544)もご参照い
ただければなおよろしいかと思います。
問題点をいくつかに分けて書きます。
- Mandrakeのパッケージを取ってきたのですが、これが
1.1.1-a017ベースで、しかもjserverに(予想通り)root setuidさ
れています。これは個人的にはパッケージ作成者に理由を聞きた
いところです。
＃せめてRedHat並みにsetuidをはずしていれば…。それでも危険
＃なのは変わりありませんが。
- (デフォルトで)setuid wnnされているのは誰でも起動・終了で
きるようにするためか、あるいは落ちやすいものだと分かってい
たからなのか、どうしてなんでしょう…。
＃JS_KILLを無効にして、元木さんのlibwrapパッチの残りを適用
＃して、innwatchみたいな監視デーモンを仕立て上げる(^^;; こ
＃とができるでしょうか…。そうしたらsetuidをはずしましょうか。
- 1.1.1-a017のころだと、 -s オプションで上書きされてしまう
のでなお危険でした。currentなソース(a019以降くらい?)では
appendモードで開くようにしたのでまだましになってます。しか
しSecurity Focus Newsletterでは「既存ファイルのチェックが
不十分である」と記述されていますので、もう少しまっとうな処
理が必要ということでしょうか。
＃shadowファイルって、ログの日付とか余計な行が入っていても
＃大丈夫なのかしら。
- でも確かに環境名を無批判に記録するのはよくないと思ったの
で、環境名とJS_OPENでのユーザ名・ホスト名をチェックするコー
ドを追加してみました。末尾に付けておきます。currentなソー
スが対象なので、昔の分で適用できなかったらごめんなさい。
- ----
たびたびML管理者の方々にお手数かけるのですが、件のデータベー
スでは「A vulnerability has been reported for FreeWnn」と
おっしゃっているので、もし届いていたら(差し支えない範囲で)
お見せいただけますか。
ところで(メンバー以外からの投稿を再び許可する代わりに)
SpamAssassinなんかと併用して管理者の負荷を下げるとか考えな
いといけないでしょうか。仕事的にも少し気になっているので
ちょっとは調べているのですが、なかなかそのものずばりという
例が見つかりませんね。
＃Googleで引っ掛けた例:
＃http://kuru.jp/index.php?%5B%5Bfml%A4%C8spamassasin%A4%CE%CF%A2%B7%C8%5D%5D
＃http://www.mew.org/ml/mew-win32-2.0/msg00514.html (その
＃MLではなく、emacs-w3m MLの話です)
----
青野智樹 (aono@XXXX)
Personal opinion only...
Index: Wnn/jserver/do_env.c
===================================================================
RCS file: /cvs/freewnn/FreeWnn/Wnn/jserver/do_env.c,v
retrieving revision 1.8
diff -u -r1.8 do_env.c
--- Wnn/jserver/do_env.c 11 May 2003 18:27:41 -0000 1.8
+++ Wnn/jserver/do_env.c 12 Aug 2003 12:15:58 -0000
@@ -57,6 +57,7 @@
static int find_env_by_name (char *);
static int find_env_in_client (int);
static void new_env (int env_id, char *n);
+static int escape_strncpy(char *dst, const char *src, size_t len);
static int sticky_cnt = 0;
static int sticky_time = 0;
@@ -75,13 +76,48 @@
register int i;
*/
char tmp_buf[256];
+ char *ptr;
+ int valid;
version = get4_cur ();
+ /* host_name */
gets_cur (tmp_buf, WNN_HOSTLEN);
strcpy (c_c->host_name, tmp_buf);
+ escape_strncpy(tmp_buf, c_c->host_name, sizeof(tmp_buf));
+ /* string validation: We should check more (ex. reverse host lookup)? */
+ valid = 1;
+ ptr = c_c->host_name;
+ for(; *ptr != '\0'; ptr++) {
+ /* Rough check: We should comply with some RFC ... */
+ if(! isalnum(*ptr) && strchr(".-_", *ptr) == NULL)
+ {
+ valid = 0;
+ *ptr = '_';
+ }
+ }
+ if(valid != 1)
+ {
+ log_err("JS_OPEN: Warning: Specified hostname (\"%s\") contains unwanted character. (Possible attack? but continue.)", tmp_buf);
+ }
+ /* user_name */
gets_cur (tmp_buf, WNN_ENVNAME_LEN);
strcpy (c_c->user_name, tmp_buf);
- error1 ("Inet user=%s@%s\n", c_c->user_name, c_c->host_name);
+ escape_strncpy(tmp_buf, c_c->user_name, sizeof(tmp_buf));
+ /* string validation: We should check more? */
+ valid = 1;
+ ptr = c_c->user_name;
+ for(; *ptr != '\0'; ptr++) {
+ if(! isalnum(*ptr))
+ {
+ valid = 0;
+ *ptr = '_';
+ }
+ }
+ if(valid != 1)
+ {
+ log_err("JS_OPEN: Warning: Specified username (\"%s\") contains unwanted character. (Possible attack? but continue.)", tmp_buf);
+ }
+ log_debug("Inet user=%s@%s\n", c_c->user_name, c_c->host_name);
/* Moved to new_client, because del_client() will be called
by longjmp before this initialization. By Kuwari
for(i=0;i<WNN_MAX_ENV_OF_A_CLIENT;i++){
@@ -157,11 +193,19 @@
putc_purge ();
}
+/*
+ Security issue (ja): n について制御文字などが入った際の措置について
+ 考慮する必要がある。今のところ n自体のチェックで拒否せず、ログの
+ 記録にとどめている。悪用を防止するため、ログに出力する制御文字は
+ 加工すること。
+ */
static int
conn1 (char *n)
{
register int eid;
register struct cnv_env *ne;
+ char n_escaped[256];
+
if ((eid = find_env_by_name (n)) != -1)
{ /* exist */
if (find_env_in_client (eid) != -1)
@@ -190,9 +234,17 @@
{
free (ne);
env[eid] = NULL;
+ log_err("conn1: Cannot add new environment to client.");
return -1;
}
- error1 ("new_env: Created , %s env_id=%d\n", n, eid);
+ escape_strncpy(n_escaped, n, sizeof(n_escaped));
+ /* (少なくともログの問題が解決するまで)そのままnを使ってはならない */
+ /* Quick check: Should we check n separately? */
+ if(strcmp(n, n_escaped) != 0)
+ {
+ log_err("conn1: Warning: Specified env string (\"%s\") contains unwanted character. (Possible attack? But continue.)", n_escaped);
+ }
+ log_debug("new_env: Created , %s env_id=%d\n", n_escaped, eid);
return eid;
}
@@ -571,4 +623,53 @@
return (eid);
#endif
return (get4_cur ());
+}
+
+/*
+ strncpy with escaping undesirable (ex. control) character.
+ But destination string should be null-terminated in this function.
+ If it's neat, you should extern this function (and maybe
+ move to other file).
+ Return value: Not 0 if all char in src is not filled in dst.
+ FIXME: It may not be 8bit safe.
+ */
+static int
+escape_strncpy(char *dst, const char *src, size_t len)
+{
+ char *dstptr = dst; /* ウォーニングをなくすためsrcはそのまま使用 */
+ char *dstend = dst + (len - 1); /* Preserve last char (for '\0') */
+ int overrun = 0;
+
+ if(len == 0 || dst == NULL || src == NULL) return 0;
+ while(*src != '\0' && dstptr < dstend) {
+ if(*src == '\\') {
+ /* Escape backslash as '\\' */
+ if(dstptr + 2 > dstend) {
+ /* Buffer exceeded: Give up */
+ overrun = 1;
+ break;
+ } else {
+ *dstptr++ = '\\';
+ *dstptr++ = '\\';
+ }
+ } else if(isgraph(*src)) {
+ *dstptr++ = *src++;
+ } else {
+ /* Escape this char: like "\xFF" */
+ if (dstptr + 4 > dstend) {
+ /* Buffer exceeded: Give up */
+ overrun = 1;
+ break;
+ } else {
+ sprintf(dstptr, "\\x%02X", *src++); /* Take care of buffer length */
+ dstptr += 4;
+ }
+ }
+ }
+ if(*src != '\0') {
+ overrun = 1;
+ }
+ *dstptr = '\0';
+
+ return overrun;
}