Merge pull request #153 from rhenium/ky/ssl-read-fix-leak-uninitialized

ssl: prevent SSLSocket#sysread* from leaking uninitialized data

2 files changed, 39 insertions, 11 deletions

diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index bf40c5b1..aa2dbbc8 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1688,20 +1688,26 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
     }
 
     ilen = NUM2INT(len);
-    if(NIL_P(str)) str = rb_str_new(0, ilen);
-    else{
-        StringValue(str);
-        rb_str_modify(str);
-        rb_str_resize(str, ilen);
+    if (NIL_P(str))
+	str = rb_str_new(0, ilen);
+    else {
+	StringValue(str);
+	if (RSTRING_LEN(str) >= ilen)
+	    rb_str_modify(str);
+	else
+	    rb_str_modify_expand(str, ilen - RSTRING_LEN(str));
     }
-    if(ilen == 0) return str;
+    OBJ_TAINT(str);
+    rb_str_set_len(str, 0);
+    if (ilen == 0)
+	return str;
 
     GetSSL(self, ssl);
     io = rb_attr_get(self, id_i_io);
     GetOpenFile(io, fptr);
     if (ssl_started(ssl)) {
 	for (;;){
-	    nread = SSL_read(ssl, RSTRING_PTR(str), RSTRING_LENINT(str));
+	    nread = SSL_read(ssl, RSTRING_PTR(str), ilen);
 	    switch(ssl_get_error(ssl, nread)){
 	    case SSL_ERROR_NONE:
 		goto end;
@@ -1751,8 +1757,6 @@ ossl_ssl_read_internal(int argc, VALUE *argv, VALUE self, int nonblock)
 
   end:
     rb_str_set_len(str, nread);
-    OBJ_TAINT(str);
-
     return str;
 }
 
diff --git a/test/test_pair.rb b/test/test_pair.rb
index 7daa9288..ea5f0dcf 100644
--- a/test/test_pair.rb
+++ b/test/test_pair.rb
@@ -218,7 +218,7 @@ module OpenSSL::TestPairM
       assert_nothing_raised("[ruby-core:20298]") { ret = s2.read_nonblock(10) }
       assert_equal("def\n", ret)
       s1.close
-      sleep 0.1
+      IO.select([s2])
       assert_raise(EOFError) { s2.read_nonblock(10) }
     }
   end
@@ -234,11 +234,35 @@ module OpenSSL::TestPairM
       assert_nothing_raised("[ruby-core:20298]") { ret = s2.read_nonblock(10, exception: false) }
       assert_equal("def\n", ret)
       s1.close
-      sleep 0.1
+      IO.select([s2])
       assert_equal(nil, s2.read_nonblock(10, exception: false))
     }
   end
 
+  def test_read_with_outbuf
+    ssl_pair { |s1, s2|
+      s1.write("abc\n")
+      buf = ""
+      ret = s2.read(2, buf)
+      assert_same ret, buf
+      assert_equal "ab", ret
+
+      buf = "garbage"
+      ret = s2.read(2, buf)
+      assert_same ret, buf
+      assert_equal "c\n", ret
+
+      buf = "garbage"
+      assert_equal :wait_readable, s2.read_nonblock(100, buf, exception: false)
+      assert_equal "", buf
+
+      s1.close
+      buf = "garbage"
+      assert_equal nil, s2.read(100, buf)
+      assert_equal "", buf
+    }
+  end
+
   def test_write_nonblock
     ssl_pair {|s1, s2|
       assert_equal 3, s1.write_nonblock("foo")