[CODE] Serious bug in do_dc()

From: Andrey Fidrya (andrey@alex-ua.com)
Date: 06/28/99


There is serious bug in do_dc.The symptoms are:

1) Pfile corruption:
Someone tries to create new character that was deleted a while ago.
So, all structures are reset, but pfilepos() is initialized to old char's
pfile address. Then "Did I get that right, name?" question is asked.
Player answers 'No' and his name is nulled. Then any immortal
disconnects player and empty non accessible entry will be saved
to pfile!

2) Crashes (related to 1st one):
Gdb sometimes shown that game has crashed in char_to_store,
which was called from save_char: "ch->player" unexistant.

3) Errors in log:
Every time immortal tries to disconnect someone currently not in
CON_PLAYING state, "SYSERR" message will appear in log.

4) Memory leak:
Every char disconnected while not in CON_PLAYING state will
create a memory leak.

For example:

Start logging: 28.06.1999 18:44
> users
Num Class   Name         State          Idl Login@   Site
--- ------- ------------ -------------- --- -------- -----------------
  2    -    Testa        Confirm name       18:44:05 [127.0.0.1]
  1 [34 Mu] Zmey         Playing            18:43:57 [127.0.0.1]

2 visible sockets connected.

> dc 2
Connection #2 closed.

>
[ Closing link to: Testa. ]

syslog:
SYSERR: no valid target to act()!
........
At first, I thought that the bug is here:
===
    if (STATE(d) == CON_PLAYING || STATE(d) == CON_DISCONNECT) {
      act("$n has lost $s link.", TRUE, d->character, 0, 0, TO_ROOM);
      if (!IS_NPC(d->character)) {
        save_char(d->character, NOWHERE);
        sprintf(buf, "Closing link to: %s.", GET_NAME(d->character));
        mudlog(buf, NRM, MAX(LVL_IMMORT, GET_INVIS_LEV(d->character)),
TRUE);
      }
      d->character->desc = NULL;
    } else {
      sprintf(buf, "Losing player: %s.",
       GET_NAME(d->character) ? GET_NAME(d->character) : "<null>");
      mudlog(buf, CMP, LVL_IMMORT, TRUE);
      free_char(d->character);
    }
===

But actual bug is in act.wizard.c, do_dc():
===
   * It is a much more logical extension for a CON_DISCONNECT to be used
   * for in-game socket closes and CON_CLOSE for out of game closings.
   * This will retain the stability of the close_me hack while being
   * neater in appearance. -gg 12/1/97
   */
+ /* Zmey: Yeah, thats right :) */
- STATE(d) = CON_DISCONNECT;
+ if (STATE(d) == CON_PLAYING)
+   STATE(d) = CON_DISCONNECT;
+ /* Is it possible to catch player being disconnected? Imho yes. */
+ else if (STATE(d) != CON_DISCONNECT && STATE(d) != CON_CLOSE)
+   STATE(d) = CON_CLOSE;
+ else {
+   send_to_char("Enough for him... ;-)\r\n", ch);
+   return;
+ }
===


This bug could be detected at once if proper sanity checks exist
in save_char():
(Apply my linkdead chars saving fix before applying this one):
===
void save_char(struct char_data *ch)
{
  struct char_file_u st;

- if (IS_NPC(ch) || GET_PFILEPOS(ch) < 0)
-   return;
+ if (IS_NPC(ch)) {
+   log("SYSERR: save_char: Trying to save NPC!");
+   return;
+ }
+ if (GET_PFILEPOS(ch) < 0) {
+   log("SYSERR: save_char: Pfilepos is negative!");
+   return;
+ }
===

Zmey // 3MoonsWorld (rmud.net.ru:4000)


     +------------------------------------------------------------+
     | Ensure that you have read the CircleMUD Mailing List FAQ:  |
     |  http://qsilver.queensu.ca/~fletchra/Circle/list-faq.html  |
     +------------------------------------------------------------+



This archive was generated by hypermail 2b30 : 12/15/00 PST