From 7a86796bb7cca96638ff55190ec06a147c63ef35 Mon Sep 17 00:00:00 2001 From: Greg Hurrell Date: Sun, 25 May 2014 11:05:56 -0700 Subject: [PATCH] Guard against overflow when peeking at PDU size This is a partial cherry-pick from my commit to the Facebook Watchman project[0]. I noticed while reviewing the corresponding pull request[1] that a malformed header could cause us to index beyond the range of the `sizes` array, so this commit pulls in the equivalent guard. Note there is a bunch of other stuff in that pull-request and that specific commit which I am not pulling down here. That code is in a gem and so needs to be more defensive; in Command-T itself, if something goes wrong with Watchman we want to fail noisily and loudly. So we're not going to worry about freeing memory before throwing an exception (effectively crashing) nor do we care about restoring the flags to their pre-`fcntl` state (as we're the only users of the socket). [0]: https://github.com/facebook/watchman/commit/5b0aa521ae [1]: https://github.com/facebook/watchman/pull/37 [2]: https://github.com/facebook/watchman/pull/37#discussion_r12498885 Signed-off-by: Greg Hurrell --- ruby/command-t/watchman.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ruby/command-t/watchman.c b/ruby/command-t/watchman.c index dd7214b1..e274f762 100644 --- a/ruby/command-t/watchman.c +++ b/ruby/command-t/watchman.c @@ -603,6 +603,7 @@ VALUE CommandTWatchmanUtils_query(VALUE self, VALUE query, VALUE socket) { int fileno, flags; int8_t peek[WATCHMAN_PEEK_BUFFER_SIZE]; int8_t sizes[] = { 0, 0, 0, 1, 2, 4, 8 }; + int8_t sizes_idx; int8_t *pdu_size_ptr; int64_t payload_size; long query_len; @@ -637,8 +638,12 @@ VALUE CommandTWatchmanUtils_query(VALUE self, VALUE query, VALUE socket) { } // peek at size of PDU + sizes_idx = peek[sizeof(WATCHMAN_BINARY_MARKER) - 1]; + if (sizes_idx < WATCHMAN_INT8_MARKER || sizes_idx > WATCHMAN_INT64_MARKER) { + rb_raise(rb_eRuntimeError, "bad PDU size marker"); + } peek_size = sizeof(WATCHMAN_BINARY_MARKER) - 1 + sizeof(int8_t) + - sizes[peek[sizeof(WATCHMAN_BINARY_MARKER) - 1]]; + sizes[sizes_idx]; received = recv(fileno, peek, peek_size, MSG_PEEK); if (received == -1) {