From d3fde50de7eb4b0df8dc98e0d21df233be2d29c2 Mon Sep 17 00:00:00 2001 From: akr Date: Sun, 4 Jun 2006 11:45:09 +0000 Subject: [PATCH] * ext/socket/socket.c: fix sockaddr_un handling. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@10212 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 4 ++ ext/socket/socket.c | 69 +++++++++++++-------- test/socket/test_unix.rb | 128 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 175 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 595298005f..99222fc41a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +Sun Jun 4 20:40:19 2006 Tanaka Akira + + * ext/socket/socket.c: fix sockaddr_un handling. + Fri Jun 2 19:00:40 2006 GOTOU Yuuzou * ext/openssl/extconf.rb: use create_header. diff --git a/ext/socket/socket.c b/ext/socket/socket.c index 299de63818..0d0b4b3d06 100644 --- a/ext/socket/socket.c +++ b/ext/socket/socket.c @@ -543,7 +543,7 @@ bsock_do_not_reverse_lookup_set(sock, state) static VALUE ipaddr _((struct sockaddr*, int)); #ifdef HAVE_SYS_UN_H -static VALUE unixaddr _((struct sockaddr_un*)); +static VALUE unixaddr _((struct sockaddr_un*, socklen_t)); #endif enum sock_recv_type { @@ -620,10 +620,7 @@ s_recvfrom(sock, argc, argv, from) #ifdef HAVE_SYS_UN_H case RECV_UNIX: - if (alen) /* connection-oriented socket may not return a from result */ - return rb_assoc_new(str, unixaddr((struct sockaddr_un*)buf)); - else - return rb_assoc_new(str, Qnil); + return rb_assoc_new(str, unixaddr((struct sockaddr_un*)buf, alen)); #endif case RECV_SOCKET: return rb_assoc_new(str, rb_str_new(buf, alen)); @@ -685,8 +682,7 @@ s_recvfrom_nonblock(int argc, VALUE *argv, VALUE sock, enum sock_recv_type from) #ifdef HAVE_SYS_UN_H case RECV_UNIX: - if (alen) /* connection-oriented socket may not return a from result */ - addr = unixaddr((struct sockaddr_un*)buf); + addr = unixaddr((struct sockaddr_un*)buf, alen); break; #endif @@ -1607,8 +1603,11 @@ init_unixsock(sock, path, server) MEMZERO(&sockaddr, struct sockaddr_un, 1); sockaddr.sun_family = AF_UNIX; - strncpy(sockaddr.sun_path, RSTRING(path)->ptr, sizeof(sockaddr.sun_path)-1); - sockaddr.sun_path[sizeof(sockaddr.sun_path)-1] = '\0'; + if (sizeof(sockaddr.sun_path) <= RSTRING(path)->len) { + rb_raise(rb_eArgError, "too long unix socket path (max: %dbytes)", + (int)sizeof(sockaddr.sun_path)-1); + } + strcpy(sockaddr.sun_path, StringValueCStr(path)); if (server) { status = bind(fd, (struct sockaddr*)&sockaddr, sizeof(sockaddr)); @@ -1634,7 +1633,9 @@ init_unixsock(sock, path, server) init_sock(sock, fd); GetOpenFile(sock, fptr); - fptr->path = strdup(RSTRING(path)->ptr); + if (server) { + fptr->path = strdup(RSTRING(path)->ptr); + } return sock; } @@ -1866,6 +1867,15 @@ unix_init(sock, path) return init_unixsock(sock, path, 0); } +static char * +unixpath(struct sockaddr_un *sockaddr, socklen_t len) +{ + if (sockaddr->sun_path < (char*)sockaddr + len) + return sockaddr->sun_path; + else + return ""; +} + static VALUE unix_path(sock) VALUE sock; @@ -1878,7 +1888,7 @@ unix_path(sock) socklen_t len = sizeof(addr); if (getsockname(fptr->fd, (struct sockaddr*)&addr, &len) < 0) rb_sys_fail(0); - fptr->path = strdup(addr.sun_path); + fptr->path = strdup(unixpath(&addr, len)); } return rb_str_new2(fptr->path); } @@ -2177,7 +2187,7 @@ unix_accept_nonblock(sock) VALUE sock; { OpenFile *fptr; - struct sockaddr_storage from; + struct sockaddr_un from; socklen_t fromlen; GetOpenFile(sock, fptr); @@ -2201,11 +2211,12 @@ unix_sysaccept(sock) #ifdef HAVE_SYS_UN_H static VALUE -unixaddr(sockaddr) +unixaddr(sockaddr, len) struct sockaddr_un *sockaddr; + socklen_t len; { return rb_assoc_new(rb_str_new2("AF_UNIX"), - rb_str_new2(sockaddr->sun_path)); + rb_str_new2(unixpath(sockaddr, len))); } #endif @@ -2221,9 +2232,7 @@ unix_addr(sock) if (getsockname(fptr->fd, (struct sockaddr*)&addr, &len) < 0) rb_sys_fail("getsockname(2)"); - if (len == 0) - addr.sun_path[0] = '\0'; - return unixaddr(&addr); + return unixaddr(&addr, len); } static VALUE @@ -2238,9 +2247,7 @@ unix_peeraddr(sock) if (getpeername(fptr->fd, (struct sockaddr*)&addr, &len) < 0) rb_sys_fail("getsockname(2)"); - if (len == 0) - addr.sun_path[0] = '\0'; - return unixaddr(&addr); + return unixaddr(&addr, len); } #endif @@ -3516,11 +3523,17 @@ sock_s_pack_sockaddr_un(self, path) VALUE self, path; { struct sockaddr_un sockaddr; + char *sun_path; VALUE addr; MEMZERO(&sockaddr, struct sockaddr_un, 1); sockaddr.sun_family = AF_UNIX; - strncpy(sockaddr.sun_path, StringValuePtr(path), sizeof(sockaddr.sun_path)-1); + sun_path = StringValueCStr(path); + if (sizeof(sockaddr.sun_path) <= strlen(sun_path)) { + rb_raise(rb_eArgError, "too long unix socket path (max: %dbytes)", + (int)sizeof(sockaddr.sun_path)-1); + } + strncpy(sockaddr.sun_path, sun_path, sizeof(sockaddr.sun_path)-1); addr = rb_str_new((char*)&sockaddr, sizeof(sockaddr)); OBJ_INFECT(addr, path); @@ -3532,15 +3545,21 @@ sock_s_unpack_sockaddr_un(self, addr) VALUE self, addr; { struct sockaddr_un * sockaddr; + char *sun_path; VALUE path; sockaddr = (struct sockaddr_un*)StringValuePtr(addr); - if (RSTRING(addr)->len != sizeof(struct sockaddr_un)) { - rb_raise(rb_eTypeError, "sockaddr_un size differs - %ld required; %d given", + if (sizeof(struct sockaddr_un) < RSTRING(addr)->len) { + rb_raise(rb_eTypeError, "too long sockaddr_un - %ld longer than %d", RSTRING(addr)->len, sizeof(struct sockaddr_un)); } - /* xxx: should I check against sun_path size? */ - path = rb_str_new2(sockaddr->sun_path); + sun_path = unixpath(sockaddr, RSTRING(addr)->len); + if (sizeof(struct sockaddr_un) == RSTRING(addr)->len && + sun_path == sockaddr->sun_path && + sun_path + strlen(sun_path) == RSTRING(addr)->ptr + RSTRING(addr)->len) { + rb_raise(rb_eArgError, "sockaddr_un.sun_path not NUL terminated"); + } + path = rb_str_new2(sun_path); OBJ_INFECT(path, addr); return path; } diff --git a/test/socket/test_unix.rb b/test/socket/test_unix.rb index b1c0a38537..081100073a 100644 --- a/test/socket/test_unix.rb +++ b/test/socket/test_unix.rb @@ -1,9 +1,11 @@ begin require "socket" - require "test/unit" rescue LoadError end +require "test/unit" +require "tempfile" + class TestUNIXSocket < Test::Unit::TestCase def test_fd_passing r1, w = IO.pipe @@ -27,4 +29,128 @@ class TestUNIXSocket < Test::Unit::TestCase r2.close if r2 && !r2.closed? end end + + def bound_unix_socket(klass) + tmpfile = Tempfile.new("testrubysock") + path = tmpfile.path + tmpfile.close(true) + yield klass.new(path), path + ensure + File.unlink path if path && File.socket?(path) + end + + def test_addr + bound_unix_socket(UNIXServer) {|serv, path| + c = UNIXSocket.new(path) + s = serv.accept + assert_equal(["AF_UNIX", path], c.peeraddr) + assert_equal(["AF_UNIX", ""], c.addr) + assert_equal(["AF_UNIX", ""], s.peeraddr) + assert_equal(["AF_UNIX", path], s.addr) + assert_equal(path, s.path) + assert_equal("", c.path) + } + end + + def test_noname_path + s1, s2 = UNIXSocket.pair + assert_equal("", s1.path) + assert_equal("", s2.path) + ensure + s1.close + s2.close + end + + def test_noname_addr + s1, s2 = UNIXSocket.pair + assert_equal(["AF_UNIX", ""], s1.addr) + assert_equal(["AF_UNIX", ""], s2.addr) + ensure + s1.close + s2.close + end + + def test_noname_peeraddr + s1, s2 = UNIXSocket.pair + assert_equal(["AF_UNIX", ""], s1.peeraddr) + assert_equal(["AF_UNIX", ""], s2.peeraddr) + ensure + s1.close + s2.close + end + + def test_noname_unpack_sockaddr_un + s1, s2 = UNIXSocket.pair + assert_equal("", Socket.unpack_sockaddr_un(s1.getsockname)) + assert_equal("", Socket.unpack_sockaddr_un(s2.getsockname)) + assert_equal("", Socket.unpack_sockaddr_un(s1.getpeername)) + assert_equal("", Socket.unpack_sockaddr_un(s2.getpeername)) + ensure + s1.close + s2.close + end + + def test_noname_recvfrom + s1, s2 = UNIXSocket.pair + s2.write("a") + assert_equal(["a", ["AF_UNIX", ""]], s1.recvfrom(10)) + ensure + s1.close + s2.close + end + + def test_noname_recvfrom_nonblock + s1, s2 = UNIXSocket.pair + s2.write("a") + IO.select [s1] + assert_equal(["a", ["AF_UNIX", ""]], s1.recvfrom_nonblock(10)) + ensure + s1.close + s2.close + end + + def test_too_long_path + assert_raise(ArgumentError) { Socket.sockaddr_un("a" * 300) } + assert_raise(ArgumentError) { UNIXServer.new("a" * 300) } + end + + def test_nul + assert_raise(ArgumentError) { Socket.sockaddr_un("a\0b") } + assert_raise(ArgumentError) { UNIXServer.new("a\0b") } + end + + def test_dgram_pair + s1, s2 = UNIXSocket.pair(Socket::SOCK_DGRAM) + assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) } + s2.send("", 0) + s2.send("haha", 0) + s2.send("", 0) + s2.send("", 0) + assert_equal("", s1.recv(10)) + assert_equal("haha", s1.recv(10)) + assert_equal("", s1.recv(10)) + assert_equal("", s1.recv(10)) + assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) } + ensure + s1.close + s2.close + end + + def test_seqpacket_pair + s1, s2 = UNIXSocket.pair(Socket::SOCK_SEQPACKET) + assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) } + s2.send("", 0) + s2.send("haha", 0) + s2.send("", 0) + s2.send("", 0) + assert_equal("", s1.recv(10)) + assert_equal("haha", s1.recv(10)) + assert_equal("", s1.recv(10)) + assert_equal("", s1.recv(10)) + assert_raise(Errno::EAGAIN) { s1.recvfrom_nonblock(10) } + ensure + s1.close + s2.close + end + end if defined?(UNIXSocket)