[IRCServices] Allocation error: ns unsuspend [retry]

[Chawmp]

Hi again,

[ I sent this yesterday, but it seems it got put under another thread in the
archive, and didn't actually get sent out to the list, so I'll send it again
in case it gets missed :) ]

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