Close Denial of Service issue with TCPConnection.expect

CHANGELOG.md

@@ -11,6 +11,7 @@ All notable changes to the Pony compiler and standard library will be documented
 - Cleanup TCPConnection GC-safety mechanism for writev buffers ([PR #3177](https://github.com/ponylang/ponyc/pull/3177))
 - Add SSL tests and fix some SSL related bugs ([PR #3174](https://github.com/ponylang/ponyc/pull/3174))
 - Fix lib/llvm to support MacOS ([PR #3181](https://github.com/ponylang/ponyc/pull/3181))
+- Close Denial of Service issue with TCPConnection.expect ([PR #3197](https://github.com/ponylang/ponyc/pull/3197))
 
 ### Added
 
@@ -26,6 +27,7 @@ All notable changes to the Pony compiler and standard library will be documented
 - Allow use of runtime TCP connect without ASIO one shot ([PR #3171](https://github.com/ponylang/ponyc/pull/3171))
 - Simplify buffering in TCPConnection ([PR #3185](https://github.com/ponylang/ponyc/pull/3185))
 - Allow for fine-grained control of yielding CPU in TCPConnection ([PR #3197](https://github.com/ponylang/ponyc/pull/3187))
+- Make TCPConnection.expect partial ([PR #3197](https://github.com/ponylang/ponyc/pull/3197))
 
 ## [0.28.1] - 2019-06-01
 

packages/net/_test.pony

@@ -12,6 +12,7 @@ actor Main is TestList
     end
     test(_TestTCPWritev)
     test(_TestTCPExpect)
+    test(_TestTCPExpectOverBufferSize)
     test(_TestTCPMute)
     test(_TestTCPUnmute)
     ifdef not windows then
@@ -217,12 +218,29 @@ class iso _TestTCPExpect is UnitTest
   fun exclusion_group(): String => "network"
 
   fun ref apply(h: TestHelper) =>
+    h.expect_action("client connect")
     h.expect_action("client receive")
     h.expect_action("server receive")
     h.expect_action("expect received")
 
     _TestTCP(h)(_TestTCPExpectNotify(h, false), _TestTCPExpectNotify(h, true))
 
+class iso _TestTCPExpectOverBufferSize is UnitTest
+  """
+  Test expecting framed data with TCP.
+  """
+  fun name(): String => "net/TCP.expectoverbuffersize"
+  fun label(): String => "unreliable-osx"
+  fun exclusion_group(): String => "network"
+
+  fun ref apply(h: TestHelper) =>
+    h.expect_action("client connect")
+    h.expect_action("connected")
+    h.expect_action("accepted")
+
+    _TestTCP(h)(_TestTCPExpectOverBufferSizeNotify(h),
+      _TestTCPExpectOverBufferSizeNotify(h))
+
 primitive _SSLContext
   fun val apply(h: TestHelper): (SSL iso^, SSL iso^) ? =>
     let sslctx =
@@ -282,16 +300,24 @@ class _TestTCPExpectNotify is TCPConnectionNotify
 
   fun ref accepted(conn: TCPConnection ref) =>
     conn.set_nodelay(true)
-    conn.expect(_expect)
-    _send(conn, "hi there")
+    try
+      conn.expect(_expect)?
+      _send(conn, "hi there")
+    else
+      _h.fail("expect threw an error")
+    end
 
   fun ref connect_failed(conn: TCPConnection ref) =>
     _h.fail_action("client connect failed")
 
   fun ref connected(conn: TCPConnection ref) =>
     _h.complete_action("client connect")
     conn.set_nodelay(true)
-    conn.expect(_expect)
+    try
+      conn.expect(_expect)?
+    else
+      _h.fail("expect threw an error")
+    end
 
   fun ref expect(conn: TCPConnection ref, qty: USize): USize =>
     _h.complete_action("expect received")
@@ -326,7 +352,11 @@ class _TestTCPExpectNotify is TCPConnectionNotify
       _expect = 4
     end
 
-    conn.expect(_expect)
+    try
+      conn.expect(_expect)?
+    else
+      _h.fail("expect threw an error")
+    end
     true
 
   fun ref _send(conn: TCPConnection ref, data: String) =>
@@ -343,6 +373,35 @@ class _TestTCPExpectNotify is TCPConnectionNotify
     buf.append(data)
     conn.write(consume buf)
 
+class _TestTCPExpectOverBufferSizeNotify is TCPConnectionNotify
+  let _h: TestHelper
+  let _expect: USize = 6_000_000_000
+
+  new iso create(h: TestHelper) =>
+    _h = h
+
+  fun ref connect_failed(conn: TCPConnection ref) =>
+    _h.fail_action("client connect failed")
+
+  fun ref accepted(conn: TCPConnection ref) =>
+    conn.set_nodelay(true)
+    try
+      conn.expect(_expect)?
+      _h.fail("expect didn't error out")
+    else
+      _h.complete_action("accepted")
+    end
+
+  fun ref connected(conn: TCPConnection ref) =>
+    _h.complete_action("client connect")
+    conn.set_nodelay(true)
+    try
+      conn.expect(_expect)?
+      _h.fail("expect didn't error out")
+    else
+      _h.complete_action("connected")
+    end
+
 class iso _TestTCPWritev is UnitTest
   """
   Test writev (and sent/sentv notification).

packages/net/tcp_connection.pony

@@ -463,14 +463,23 @@ actor TCPConnection
     @pony_os_peername[Bool](_fd, ip)
     ip
 
-  fun ref expect(qty: USize = 0) =>
+  fun ref expect(qty: USize = 0) ? =>
     """
     A `received` call on the notifier must contain exactly `qty` bytes. If
     `qty` is zero, the call can contain any amount of data. This has no effect
     if called in the `sent` notifier callback.
+
+    Errors if `qty` exceeds the max buffer size as indicated by the
+    read_buffer_size` supplied when the connection was created.
     """
-    if not _in_sent then
-      _expect = _notify.expect(this, qty)
+
+    if qty <= _read_buffer_size then
+      if not _in_sent then
+        _expect = _notify.expect(this, qty)
+        _read_buf_size()
+      end
+    else
+      error
     end
 
   fun ref set_nodelay(state: Bool) =>