Module: ngircd.git Branch: master Commit: cde2e8a2775e8b01266627a60a08e2560eac42c8 URL: http://ngircd.barton.de/cgi-bin/gitweb.cgi?p=ngircd.git&a=commit;h=cde2e8a27... Author: Federico G. Schwindt <fgsch@lodoss.net> Date: Fri Apr 19 23:51:42 2013 +0100 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. --- src/ngircd/class.c | 41 +++++++++++++++++------------------------ src/ngircd/class.h | 3 ++- src/ngircd/lists.c | 22 ++++++++++++++-------- src/ngircd/lists.h | 3 ++- 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/ngircd/class.c b/src/ngircd/class.c index 0f617b8..800e993 100644 --- a/src/ngircd/class.c +++ b/src/ngircd/class.c @@ -33,8 +33,6 @@ struct list_head My_Classes[CLASS_COUNT]; -char Reject_Reason[COMMAND_LEN]; - GLOBAL void Class_Init(void) { @@ -49,32 +47,29 @@ Class_Exit(void) 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); - 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: - snprintf(Reject_Reason, sizeof(Reject_Reason), - "\"%s\" (G-Line)", reason); - return Reject_Reason; + snprintf(reason, len, "\"%s\" (G-Line)", str); + break; 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) { - char *rejectptr; + char reject[COMMAND_LEN]; 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; } diff --git a/src/ngircd/class.h b/src/ngircd/class.h index 2a9dbba..ba2e160 100644 --- 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)); -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)); diff --git a/src/ngircd/lists.c b/src/ngircd/lists.c index 21058a0..bab30f2 100644 --- 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) { - free(e->reason); + if (e->reason) + free(e->reason); e->reason = strdup(Reason); } return true; @@ -320,18 +321,21 @@ Lists_MakeMask(const char *Pattern) 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 reason Result buffer to store the reason. + * @param len Size of the buffer. * @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; @@ -343,19 +347,21 @@ Lists_CheckReason(struct list_head *h, CLIENT *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); } - return e->reason ? e->reason : ""; + return true; } last = e; e = next; } - return NULL; + return false; } /** diff --git a/src/ngircd/lists.h b/src/ngircd/lists.h index 24504df..eb863db 100644 --- 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 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));