[IRCServices] Allocation error: ns unsuspend

Chawmp chawmp at cyberarmy.net
Sun Jul 25 13:20:28 PDT 2004


Hi again,

I just noticed and patched another bug, quite similar to the last issue I
posted about ("Seg Fault/Bus Error on SQUIT":
http://www.ircservices.za.net/pipermail/ircservices/2004/003377.html ).

Again, I noticed this one because free() is set up to write a pattern over
memory as it is released on our services host. It wouldn't occur in most
normal circumstances, but might do unpredictably, depending on a lot of
factors, so ought to be fixed.

The problem occurs when unsuspending a nickname that is part of a group in
which no nickname has been used for longer than the NSExpire setting - or
something along those lines.

In modules/nickserv/util.c, unsuspend_nick() does this:

	ARRAY_FOREACH (i, ngi->nicks) {
		NickInfo *ni = get_nickinfo(ngi->nicks[i]);
		...
	}

get_nickinfo() will free (NickGroupInfo *)ngi under certain conditions
(roughly as I described above), making the following attempts to dereference
ngi crash the program:

		if (!ni) {
			module_log("unsuspend: unable to retrieve NickInfo
for %s"
				" (nick group %u)", ngi->nicks[i], ngi->id);
			continue;
		}

ngi would also be used in subsequent loops, so just changing the log message
wouldn't be a solution.

I didn't have a lot of time to investigate the best way to fix this, but
here is the patch I came up with. It seems to do the job, but I would be
grateful if anyone can advise something more suitable. (Lines beginning "+"
were added; noexpire is just set to 1 before the loop, and restored
afterwards, which stops the expiry check. Note that the NSSuspendGrace stuff
only happens a few lines after the call to get_nickinfo(), so for that small
time, nick groups can disappear.).

  void unsuspend_nick(NickGroupInfo *ngi, int set_time)
  {
      time_t now = time(NULL);
+ int cache_noexpire = 0;
  
      if (!ngi->suspendinfo) {
  	module_log("unsuspend: called on non-suspended nick group %u [%s]",

...

  		       " %u) to %ld", ngi->nicks[ngi->mainnick], ngi->id,
  		       (long)ngi->authset);
  	}
+ cache_noexpire = noexpire;
+ noexpire = 1;
  	ARRAY_FOREACH (i, ngi->nicks) {
  	    NickInfo *ni = get_nickinfo(ngi->nicks[i]);
  	    if (!ni) {

...

  	    }
  	}
      }
+ noexpire = cache_noexpire;
  }
  
 
/*************************************************************************/

--
Tom McIntyre
chawmp at cyberarmy.net