[IRCServices] Allocation error: ns unsuspend [retry]

[Andrew Church]

     I did see your original message--it's just that I've been going
through the entire code to find and fix similar bugs as well.  I have a
sneaking suspicion that the expire-on-get design plays nasty games with
pointers in the user record too, but as it hasn't seemed to cause any
problems so far, I'm saving that bit of contemplation for 5.1 (which,
incidentally, will also have similar use-after-free checks in the memory
checking code).

     At any rate, thanks for the report.

  --Andrew Church
    achurch at achurch.org
    http://achurch.org/

>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
>
>
>
>------------------------------------------------------------------
>To unsubscribe or change your subscription options, visit:
>http://www.ircservices.za.net/mailman/listinfo/ircservices