Skip to content

Commit

Permalink
Guard against overflow when peeking at PDU size
Browse files Browse the repository at this point in the history
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]: facebook/watchman@5b0aa521ae
[1]: facebook/watchman#37
[2]: facebook/watchman#37 (comment)

Signed-off-by: Greg Hurrell <greg@hurrell.net>
  • Loading branch information
wincent committed May 25, 2014
1 parent 0dda670 commit 7a86796
Showing 1 changed file with 6 additions and 1 deletion.
7 changes: 6 additions & 1 deletion ruby/command-t/watchman.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 7a86796

Please sign in to comment.