Merge pull request #126 from retropc/jupe_fixes

JUPE: cleanup code and fix UAF bugs

jupe/jupe.c

@@ -10,9 +10,11 @@
 #include "../lib/irc_string.h"
 #include "jupe.h"
 
-jupe_t *jupes = NULL;
+static jupe_t *jupes = NULL;
 
-int handlejupe(void *source, int cargc, char **cargv);
+static int handlejupe(void *source, int cargc, char **cargv);
+static jupe_t *make_jupe(char *server, char *reason, time_t expirets, time_t lastmod, unsigned int flags);
+static void jupe_free(jupe_t *);
 
 void _init() {
   /* If we're connected to IRC, force a disconnect. */
@@ -25,21 +27,18 @@ void _init() {
 }
 
 void _fini() {
-  jupe_t *next;
-
   while (jupes) {
-    /* keep a pointer to the next item */
-    next = jupes->ju_next;
+    jupe_t *next = jupes->ju_next;
 
-    free(jupes);
+    jupe_free(jupes);
 
     jupes = next;
   }
 
   deregisterserverhandler("JU", &handlejupe);
 }
 
-int handlejupe(void *source, int cargc, char **cargv) {
+static int handlejupe(void *source, int cargc, char **cargv) {
   char *server, *expire, *modtime, *reason;
   jupe_t *jupe;
   unsigned int flags;
@@ -115,46 +114,39 @@ void jupe_propagate(jupe_t *jupe) {
            JupeReason(jupe));
 }
 
-void jupe_expire(void) {
-  jupe_find(NULL);
-}
-
-jupe_t *jupe_find(char *server) {
-  jupe_t *jupe = jupes;
+static void jupe_expire(void) {
+  time_t nettime = getnettime();
 
-  while (jupe) {
-    /* server == NULL if jupe_find() is used by jupe_expire */
-    if (server && ircd_strcmp(server, JupeServer(jupe)) == 0)
-      return jupe;
-
-    if (jupe->ju_next && jupe->ju_next->ju_expire < getnettime())
-      jupe_free(jupe->ju_next);
-
-      jupe = jupe->ju_next;
+  for (jupe_t **p = &jupes, *j = *p; j; j = *p) {
+    if (j->ju_expire <= nettime) {
+      *p = j->ju_next;
+      jupe_free(j);
+    } else {
+      p = &j->ju_next;
+    }
   }
-
-  if (jupes && jupes->ju_expire < getnettime())
-    jupe_free(jupes);
-
-  return NULL;
 }
 
-void jupe_free(jupe_t *jupe) {
-  jupe_t *trav = jupes;
+jupe_t *jupe_next(jupe_t *current) {
+  if (current == NULL) {
+    jupe_expire();
+    return jupes;
+  }
 
-  if (jupe == jupes)
-    jupes = jupe->ju_next;
-  else {
-    while (trav) {
-      if (trav->ju_next == jupe) {
-        trav->ju_next = jupe->ju_next;
-        break;
-      }
+  return current->ju_next;
+}
 
-      trav = trav->ju_next;
+jupe_t *jupe_find(char *server) {
+  for (jupe_t *jupe = jupe_next(NULL); jupe; jupe = jupe_next(jupe)) {
+    if (ircd_strcmp(server, JupeServer(jupe)) == 0) {
+      return jupe;
     }
   }
 
+  return NULL;
+}
+
+static void jupe_free(jupe_t *jupe) {
   freesstring(jupe->ju_server);
   freesstring(jupe->ju_reason);
   free(jupe);

jupe/jupe.h

@@ -7,8 +7,6 @@ typedef struct jupe_s {
 	unsigned int	ju_flags;
 } jupe_t;
 
-extern jupe_t *jupes;
-
 #define JUPE_MAX_EXPIRE	604800
 
 #define JUPE_ACTIVE	0x0001
@@ -19,13 +17,8 @@ extern jupe_t *jupes;
 #define JupeReason(j)		((j)->ju_reason->content)
 #define JupeLastMod(j)		((j)->ju_lastmod)
 
-void jupe_propagate(jupe_t *jupe);
-jupe_t *make_jupe(char *server, char *reason, time_t expirets, time_t lastmod, unsigned int flags);
-void jupe_free(jupe_t *jupe);
-
-/* (public) functions for using/modifying jupes */
 jupe_t *jupe_find(char *server);
 void jupe_activate(jupe_t *jupe);
 void jupe_deactivate(jupe_t *jupe);
 int jupe_add(char *server, char *reason, time_t duration, unsigned int flags);
-void jupe_expire(void); /* call this before directly using the jupes list */
+jupe_t *jupe_next(jupe_t *current);

jupe/jupe_commands.c

@@ -13,8 +13,6 @@ int ju_addjupe(void *source, int cargc, char **cargv) {
     return CMD_USAGE;
   }
 
-  jupe_expire();
-
   if (jupe_find(cargv[0]) != NULL) {
     controlreply(np, "There is already a jupe for that server.");
     return CMD_OK;
@@ -46,8 +44,6 @@ int ju_activatejupe(void *source, int cargc, char **cargv) {
     return CMD_USAGE;
   }
 
-  jupe_expire();
-
   jupe = jupe_find(cargv[0]);
 
   if (jupe == NULL) {
@@ -76,8 +72,6 @@ int ju_deactivatejupe(void *source, int cargc, char **cargv) {
     return CMD_USAGE;
   }
 
-  jupe_expire();
-
   jupe = jupe_find(cargv[0]);
 
   if (jupe == NULL) {
@@ -99,18 +93,11 @@ int ju_deactivatejupe(void *source, int cargc, char **cargv) {
 
 int ju_jupelist(void *source, int cargc, char **cargv) {
   nick *np = (nick*)source;
-  jupe_t *jupe;
-
-  jupe_expire();
 
-  jupe = jupes;
-
   controlreply(np, "Server Reason Expires Status");
 
-  while (jupe) {
+  for (jupe_t *jupe = jupe_next(NULL); jupe; jupe = jupe_next(jupe)) {
     controlreply(np, "%s %s %s %s", JupeServer(jupe), JupeReason(jupe), longtoduration(jupe->ju_expire - getnettime(), 0), (jupe->ju_flags & JUPE_ACTIVE) ? "activated" : "deactivated");
-
-    jupe = jupe->ju_next;
   }
 
   controlreply(np, "--- End of JUPE list.");