Fix use-after-free on Lists_CheckReason()

Change Lists_CheckReason() to receive a buffer where the reason
will be stored and its length.  Change callers accordingly.

Change Class_GetMemberReason() (and its callers) in a similar way
so it doesn't rely on a global buffer for the rejected reason.

diff --git a/src/ngircd/class.c b/src/ngircd/class.c
index 0f617b8213d1ff3fe56a5cbf9babf58265080df9..800e99353647f8758488a30db2d8f41259dc1129 100644 (file)
--- a/src/ngircd/class.c
+++ b/src/ngircd/class.c
@@ -33,8 +33,6 @@
 
 struct list_head My_Classes[CLASS_COUNT];
 
 
 struct list_head My_Classes[CLASS_COUNT];
 

-char Reject_Reason[COMMAND_LEN];
-

 GLOBAL void
 Class_Init(void)
 {
 GLOBAL void
 Class_Init(void)
 {


@@ -49,32 +47,29 @@ Class_Exit(void)
        for (i = 0; i < CLASS_COUNT; Lists_Free(&My_Classes[i++]));
 }
 
        for (i = 0; i < CLASS_COUNT; Lists_Free(&My_Classes[i++]));
 }
 

-GLOBAL char *
-Class_GetMemberReason(const int Class, CLIENT *Client)
+GLOBAL bool
+Class_GetMemberReason(const int Class, CLIENT *Client, char *reason, size_t len)

 {
 {

-       char *reason;
+       char str[COMMAND_LEN] = "listed";

 
        assert(Class < CLASS_COUNT);
        assert(Client != NULL);
 
 
        assert(Class < CLASS_COUNT);
        assert(Client != NULL);
 

-       reason = Lists_CheckReason(&My_Classes[Class], Client);
-       if (!reason)
-               return NULL;
-
-       if (!*reason)
-               reason = "listed";
+       if (!Lists_CheckReason(&My_Classes[Class], Client, str, sizeof(str)))
+               return false;

 
        switch(Class) {
                case CLASS_GLINE:
 
        switch(Class) {
                case CLASS_GLINE:

-                       snprintf(Reject_Reason, sizeof(Reject_Reason),
-                                "\"%s\" (G-Line)", reason);
-                       return Reject_Reason;
+                       snprintf(reason, len, "\"%s\" (G-Line)", str);
+                       break;

                case CLASS_KLINE:
                case CLASS_KLINE:

-                       snprintf(Reject_Reason, sizeof(Reject_Reason),
-                                "\"%s\" (K-Line)", reason);
-                       return Reject_Reason;
+                       snprintf(reason, len, "\"%s\" (K-Line)", str);
+                       break;
+               default:
+                       snprintf(reason, len, "%s", str);
+                       break;

        }
        }

-       return reason;
+       return true;

 }
 
 /**
 }
 
 /**


@@ -88,15 +83,13 @@ Class_GetMemberReason(const int Class, CLIENT *Client)
 GLOBAL bool
 Class_HandleServerBans(CLIENT *Client)
 {
 GLOBAL bool
 Class_HandleServerBans(CLIENT *Client)
 {

-       char *rejectptr;
+       char reject[COMMAND_LEN];

 
        assert(Client != NULL);
 
 
        assert(Client != NULL);
 

-       rejectptr = Class_GetMemberReason(CLASS_GLINE, Client);
-       if (!rejectptr)
-               rejectptr = Class_GetMemberReason(CLASS_KLINE, Client);
-       if (rejectptr) {
-               Client_Reject(Client, rejectptr, true);
+       if (Class_GetMemberReason(CLASS_GLINE, Client, reject, sizeof(reject)) ||
+           Class_GetMemberReason(CLASS_KLINE, Client, reject, sizeof(reject))) {
+               Client_Reject(Client, reject, true);

                return DISCONNECTED;
        }
 
                return DISCONNECTED;
        }
 

diff --git a/src/ngircd/class.h b/src/ngircd/class.h
index 2a9dbba76a88db4276099e4525bab5f32fd56314..ba2e16069def9e0ad4abb365a97462cef3c46ec9 100644 (file)
--- a/src/ngircd/class.h
+++ b/src/ngircd/class.h
@@ -29,7 +29,8 @@ GLOBAL bool Class_AddMask PARAMS((const int Class, const char *Mask,
                                  const time_t ValidUntil, const char *Reason));
 GLOBAL void Class_DeleteMask PARAMS((const int Class, const char *Mask));
 
                                  const time_t ValidUntil, const char *Reason));
 GLOBAL void Class_DeleteMask PARAMS((const int Class, const char *Mask));
 

-GLOBAL char *Class_GetMemberReason PARAMS((const int Class, CLIENT *Client));
+GLOBAL bool Class_GetMemberReason PARAMS((const int Class, CLIENT *Client,
+                                         char *reason, size_t len));

 GLOBAL bool Class_HandleServerBans PARAMS((CLIENT *Client));
 
 GLOBAL struct list_head *Class_GetList PARAMS((const int Class));
 GLOBAL bool Class_HandleServerBans PARAMS((CLIENT *Client));
 
 GLOBAL struct list_head *Class_GetList PARAMS((const int Class));

diff --git a/src/ngircd/lists.c b/src/ngircd/lists.c
index 21058a03116298b6cc81ff98f40b40d0c9caa502..bab30f221be15766267347cf2a0f9f14ae5e92de 100644 (file)
--- a/src/ngircd/lists.c
+++ b/src/ngircd/lists.c
@@ -130,7 +130,8 @@ Lists_Add(struct list_head *h, const char *Mask, time_t ValidUntil,
        if (e) {
                e->valid_until = ValidUntil;
                if (Reason) {
        if (e) {
                e->valid_until = ValidUntil;
                if (Reason) {

-                       free(e->reason);
+                       if (e->reason)
+                               free(e->reason);

                        e->reason = strdup(Reason);
                }
                return true;
                        e->reason = strdup(Reason);
                }
                return true;


@@ -320,18 +321,21 @@ Lists_MakeMask(const char *Pattern)
 bool
 Lists_Check(struct list_head *h, CLIENT *Client)
 {
 bool
 Lists_Check(struct list_head *h, CLIENT *Client)
 {

-       return Lists_CheckReason(h, Client) != NULL;
+       return Lists_CheckReason(h, Client, NULL, 0);

 }
 
 /**
 }
 
 /**

- * Check if a client is listed in a list and return the "reason".
+ * Check if a client is listed in a list and store the reason if a buffer
+ * is provided.

  *
  *

- * @param h List head.
+ * @param h      List head.

  * @param Client Client to check.
  * @param Client Client to check.

+ * @param reason Result buffer to store the reason.
+ * @param len    Size of the buffer.

  * @return true if client is listed, false if not.
  */
  * @return true if client is listed, false if not.
  */

-char *
-Lists_CheckReason(struct list_head *h, CLIENT *Client)
+bool
+Lists_CheckReason(struct list_head *h, CLIENT *Client, char *reason, size_t len)

 {
        struct list_elem *e, *last, *next;
 
 {
        struct list_elem *e, *last, *next;
 


@@ -343,19 +347,21 @@ Lists_CheckReason(struct list_head *h, CLIENT *Client)
        while (e) {
                next = e->next;
                if (Match(e->mask, Client_MaskCloaked(Client))) {
        while (e) {
                next = e->next;
                if (Match(e->mask, Client_MaskCloaked(Client))) {

+                       if (len && e->reason)
+                               strlcpy(reason, e->reason, len);

                        if (e->valid_until == 1) {
                                /* Entry is valid only once, delete it */
                                LogDebug("Deleted \"%s\" from list (used).",
                                         e->mask);
                                Lists_Unlink(h, last, e);
                        }
                        if (e->valid_until == 1) {
                                /* Entry is valid only once, delete it */
                                LogDebug("Deleted \"%s\" from list (used).",
                                         e->mask);
                                Lists_Unlink(h, last, e);
                        }

-                       return e->reason ? e->reason : "";
+                       return true;

                }
                last = e;
                e = next;
        }
 
                }
                last = e;
                e = next;
        }
 

-       return NULL;
+       return false;

 }
 
 /**
 }
 
 /**

diff --git a/src/ngircd/lists.h b/src/ngircd/lists.h
index 24504dfab9d11860a2c8459b12e7ff5e22a211b0..eb863db9f0a08c02aa6740a46f2547ae3761ea5e 100644 (file)
--- a/src/ngircd/lists.h
+++ b/src/ngircd/lists.h
@@ -30,7 +30,8 @@ GLOBAL struct list_elem *Lists_GetFirst PARAMS((const struct list_head *));
 GLOBAL struct list_elem *Lists_GetNext PARAMS((const struct list_elem *));
 
 GLOBAL bool Lists_Check PARAMS((struct list_head *head, CLIENT *client));
 GLOBAL struct list_elem *Lists_GetNext PARAMS((const struct list_elem *));
 
 GLOBAL bool Lists_Check PARAMS((struct list_head *head, CLIENT *client));

-GLOBAL char *Lists_CheckReason PARAMS((struct list_head *head, CLIENT *client));
+GLOBAL bool Lists_CheckReason PARAMS((struct list_head *head, CLIENT *client,
+                                     char *reason, size_t len));

 GLOBAL struct list_elem *Lists_CheckDupeMask PARAMS((const struct list_head *head,
                                        const char *mask));
 
 GLOBAL struct list_elem *Lists_CheckDupeMask PARAMS((const struct list_head *head,
                                        const char *mask));