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

ssl: prevent SSLSocket#sysread* from leaking uninitialized data
ruby · Sep 24, 2017 · 295dd1d · 295dd1d
2 parents 45bec38 + 6ff7844
commit 295dd1d
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 11 deletions.
diff --git 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
@@ -218,7 +218,7 @@ def test_read_nonblock
  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 @@ def test_read_nonblock_no_exception
  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")