Skip to content

Commit

Permalink
Close Denial of Service issue with TCPConnection.expect
Browse files Browse the repository at this point in the history
Prior to this commit, `expect` could be used to create a denial of
service on a Pony application. If you used expect to handle a framed
protocol, then, you'd by default accept any sized `except` value which
could result in huge amounts of memory being allocated. Memory that
could vastly outstrip the max buffer size set for controlling memory
usage.

After this commit, `expect` is partial and will result in an error if
attempting to set an expect value that is greater than our max read
buffer size.

I intended to commit this prior to another PR, but forgot and thought
that I had. As it is, this PR works in conjunction with #3185 to prevent
expect from being passed that is greater than the max buffer size
thereby deadlocking the logic post #3185.
  • Loading branch information
SeanTAllen committed Jun 18, 2019
1 parent e5e0180 commit db66148
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
20 changes: 16 additions & 4 deletions packages/net/_test.pony
Original file line number Diff line number Diff line change
Expand Up @@ -282,16 +282,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
conn.close()
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
conn.close()
end

fun ref expect(conn: TCPConnection ref, qty: USize): USize =>
_h.complete_action("expect received")
Expand Down Expand Up @@ -326,7 +334,11 @@ class _TestTCPExpectNotify is TCPConnectionNotify
_expect = 4
end

conn.expect(_expect)
try
conn.expect(_expect)?
else
conn.close()
end
true

fun ref _send(conn: TCPConnection ref, data: String) =>
Expand Down
15 changes: 11 additions & 4 deletions packages/net/tcp_connection.pony
Original file line number Diff line number Diff line change
Expand Up @@ -463,15 +463,22 @@ 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.
`qty` can not exceed 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)
_read_buf_size()
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) =>
Expand Down

0 comments on commit db66148

Please sign in to comment.