[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] poll.c: ssh_poll_ctx_remove(): added security check


From 440467b0cb81d032077d14c360abc28f816f92de Mon Sep 17 00:00:00 2001
From: Younes Serraj <younes.serraj@xxxxxxxxx>
Date: Wed, 27 Jan 2016 11:13:20 +0100
Subject: [PATCH] poll.c: ssh_poll_ctx_remove(): added security check

ssh_poll_ctx_remove() removes a poll object from a context, but doesn't make
sure the said poll object is actually registered in the given context, or in
any context at all. This can lead to the poll object's fd being wrongly used
as an index in the wrong context, which can generate one of the following:
- undefined behavior (random poll object removed from it's context)
- segfault

This patch adds the necessary check to make sure the given poll object is
associated with the given context.
ssh_poll_ctx_remove() now has a return value.

Signed-off-by: Younes Serraj <younes.serraj@xxxxxxxxx>
---
 include/libssh/poll.h | 2 +-
 src/poll.c            | 8 +++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/libssh/poll.h b/include/libssh/poll.h
index bbc03a9..e3ddf3f 100644
--- a/include/libssh/poll.h
+++ b/include/libssh/poll.h
@@ -152,7 +152,7 @@ ssh_poll_ctx ssh_poll_ctx_new(size_t chunk_size);
 void ssh_poll_ctx_free(ssh_poll_ctx ctx);
 int ssh_poll_ctx_add(ssh_poll_ctx ctx, ssh_poll_handle p);
 int ssh_poll_ctx_add_socket (ssh_poll_ctx ctx, struct ssh_socket_struct *s);
-void ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p);
+int ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p);
 int ssh_poll_ctx_dopoll(ssh_poll_ctx ctx, int timeout);
 ssh_poll_ctx ssh_poll_get_default_ctx(ssh_session session);
 
diff --git a/src/poll.c b/src/poll.c
index 807b0a5..ee28cf7 100644
--- a/src/poll.c
+++ b/src/poll.c
@@ -556,10 +556,14 @@ int ssh_poll_ctx_add_socket (ssh_poll_ctx ctx, ssh_socket s) {
  *
  * @param  ctx          Pointer to an already allocated poll context.
  * @param  p            Pointer to an already allocated poll object.
+ * @return              0 on success, < 0 on error
  */
-void ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p) {
+int ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p) {
   size_t i;
 
+  if (p->ctx != ctx)
+    return -1;
+
   i = p->x.idx;
   p->x.fd = ctx->pollfds[i].fd;
   p->ctx = NULL;
@@ -577,6 +581,8 @@ void ssh_poll_ctx_remove(ssh_poll_ctx ctx, ssh_poll_handle p) {
   if (ctx->polls_allocated - ctx->polls_used > ctx->chunk_size) {
     ssh_poll_ctx_resize(ctx, ctx->polls_allocated - ctx->chunk_size);
   }
+
+  return 0;
 }
 
 /**
-- 
2.7.0.rc3


Archive administrator: postmaster@lists.cynapses.org