[ruby-dev:15849] possible buffer overrun & pathname validation

なかだです。

rb_file_s_expand_path()を見直してみたら、まだバッファオーバーラ
ンの可能性がありそうです。条件を指定するというのがどちらかとい
うと間違いやすいような気がするので、BUFCHECK()は長さを指定する
ようにしたほうがいいんじゃないかと思うのですが。

それともう一点、Stringからchar*を取り出してパス名として使う場合、
NUL terminateされているかどうかとか、途中にNULが入っていないか
などをチェックするようにしたほうがいいような気がします。


Index: file.c
===================================================================
RCS file: /cvs/ruby/src/ruby/file.c,v
retrieving revision 1.87
diff -u -2 -p -r1.87 file.c
--- file.c	2002/02/04 13:49:05	1.87
+++ file.c	2002/02/04 15:27:30
@@ -1378,12 +1378,12 @@ strrdirsep(path)
 }
 
-#define BUFCHECK(cond) while (cond) {\
+#define BUFRESIZE(needs) do {\
     long bdiff = p - buf;\
-    buflen *= 2;\
+    while (needs >= buflen) {buflen *= 2;}\
     rb_str_resize(result, buflen);\
     buf = RSTRING(result)->ptr;\
     p = buf + bdiff;\
     pend = buf + buflen;\
-}
+} while (0)
 
 VALUE
@@ -1395,4 +1395,5 @@ rb_file_s_expand_path(argc, argv)
     char *s, *buf, *b, *p, *pend;
     long buflen = MAXPATHLEN;
+    long len;
     int tainted;
 
@@ -1412,7 +1413,8 @@ rb_file_s_expand_path(argc, argv)
 		rb_raise(rb_eArgError, "couldn't find HOME environment -- expanding `%s'", s);
 	    }
-	    BUFCHECK (strlen(dir) > buflen);
+	    len = strlen(dir);
+	    BUFRESIZE (len);
 	    strcpy(buf, dir);
-	    p = buf + strlen(dir);
+	    p = buf + len;
 	    s++;
 	    tainted = 1;
@@ -1427,5 +1429,5 @@ rb_file_s_expand_path(argc, argv)
 		s = CharNext(s);
 	    }
-	    BUFCHECK (p + (s-b) >= pend);
+	    BUFRESIZE ((p-buf) + (s-b));
 	    memcpy(p, b, s-b);
 	    p += s-b;
@@ -1437,7 +1439,8 @@ rb_file_s_expand_path(argc, argv)
 		rb_raise(rb_eArgError, "user %s doesn't exist", buf);
 	    }
-	    BUFCHECK (strlen(pwPtr->pw_dir) > buflen);
+	    len = strlen(pwPtr->pw_dir);
+	    BUFRESIZE (len);
 	    strcpy(buf, pwPtr->pw_dir);
-	    p = buf + strlen(pwPtr->pw_dir);
+	    p = buf + len;
 	    endpwent();
 #endif
@@ -1451,5 +1454,5 @@ rb_file_s_expand_path(argc, argv)
 	    s = CharNext(s);
 	}
-	BUFCHECK (p + (s-b) >= pend);
+	BUFRESIZE ((p-buf) + (s-b));
 	memcpy(p, b, s-b);
 	p += s-b;
@@ -1460,5 +1463,6 @@ rb_file_s_expand_path(argc, argv)
 	    dname = rb_file_s_expand_path(1, &dname);
 	    if (OBJ_TAINTED(dname)) tainted = 1;
-	    BUFCHECK (strlen(RSTRING(dname)->ptr) > buflen);
+	    len = strlen(RSTRING(dname)->ptr);
+	    BUFRESIZE (len);
 	    strcpy(buf, RSTRING(dname)->ptr);
 	}
@@ -1467,8 +1471,9 @@ rb_file_s_expand_path(argc, argv)
 
 	    tainted = 1;
-	    BUFCHECK (strlen(dir) > buflen);
+	    len = strlen(dir);
+	    BUFRESIZE (len);
 	    strcpy(buf, dir);
 	}
-	p = &buf[strlen(buf)];
+	p = &buf[len];
 	while (p > buf && *(p - 1) == '/') p--;
     }
@@ -1476,5 +1481,5 @@ rb_file_s_expand_path(argc, argv)
 	while (*s && isdirsep(*s)) {
 	    *p++ = '/';
-	    BUFCHECK (p >= pend);
+	    BUFRESIZE (p-buf);
 	    s++;
 	}
@@ -1522,5 +1527,5 @@ rb_file_s_expand_path(argc, argv)
 #endif
 	    if (s > b) {
-		BUFCHECK (p + (s-b+1) >= pend);
+		BUFRESIZE ((p-buf) + (s-b+1));
 		memcpy(++p, b, s-b);
 		p += s-b;
@@ -1536,5 +1541,5 @@ rb_file_s_expand_path(argc, argv)
 
     if (s > b) {
-	BUFCHECK (p + (s-b) >= pend);
+	BUFRESIZE ((p-buf) + (s-b));
 	memcpy(++p, b, s-b);
 	p += s-b;


-- 
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
    中田 伸悦