[srvx-bugs] [ srvx-Bugs-2668866 ] MemoServ inconsistencies

[SourceForge.net <no...@so...>]

Bugs item #2668866, was opened at 2009-03-06 18:12
Message generated for change (Tracker Item Submitted) made by Item Submitter
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=403001&aid=2668866&group_id=31654

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Miscellaneous
Group: 1.4
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: David Herrmann (dvdhrm)
Assigned to: Nobody/Anonymous (nobody)
Summary: MemoServ inconsistencies

Initial Comment:
We tested MemoServ on our network and there are several bugs and some weird behaviour we discovered.
The line-numbers in the patches may differ slightly from srvx-1.4.0-rc3.
First, the bugs:

MemoServ uses an uninitialized timestamp in cmd_read.
A fix could probably look like:
@@ -312,6 +312,7 @@
         return 0;
     if (!(memo = find_memo(user, cmd, ma, argv[1], &memoid)))
         return 0;
+    feh = memo->sent;
     strftime(posted, sizeof(posted), "%I:%M %p, %m/%d/%Y", localtime(&feh));
     reply("MSMSG_MEMO_HEAD", memoid, memo->sender->handle->handle, posted);
     send_message_type(4, user, cmd->parent->bot, "%s", memo->message);

Another bug is, that memoserv does not save the settings of each account. We currently have
not fixed it but memoserv should simply save the "->flags" member of the memo_account structure
into the memoserv.db.

Third and last bug:
Each memo is saved in memoserv.db with a base64 ID. base64, however, is case-sensitive and saxdb
is *not*. This could be fixed by enabling a case-sensitive option in saxdb (I don't know whether such an
option exists) in the memoserv module or by saving the ID without base64 encoding.
This bug apparently causes a silent drop of messages.


Despite these bugs we wondered why it is possible to send memos to yourself.  In our opinion MemoServ should
not be used to save arbitrary messages in your account. We changed this behavior the following way:
Adding: { "MSMSG_CANNOT_SEND_SELF", "You cannot send memos to yourself." },
and:
@@ -216,6 +217,10 @@
 
     if (!user->handle_info)
         return 0;
+    if (user->handle_info == acct->handle) {
+        send_message(user, bot, "MSMSG_CANNOT_SEND_SELF");
+        return 0;
+    }
     if (!(acct->flags & MEMO_DENY_NONCHANNEL))
         return 1;
     for (dest = acct->handle->channels; dest; dest = dest->u_next)


We also added the ID of a message to the notification of new messages. This allows to read a message
without searching for the ID in the memo-list.
-    { "MSMSG_NEW_MESSAGE", "You have a new message from $b%s$b." },
+    { "MSMSG_NEW_MESSAGE", "You have a new message from $b%s$b (ID: %u)." },
And:
@@ -263,7 +268,7 @@
-            send_message(other, cmd->parent->bot, "MSMSG_NEW_MESSAGE", user->nick);
+            send_message(other, cmd->parent->bot, "MSMSG_NEW_MESSAGE", user->nick, ma->recvd.used - 1);


Furthermore, we changed memoserv to send new-message-notifications always as queries and not depend on the "privmsg" setting
of the authserv account, because many common IRC Clients have a weird NOTICE behaviour and we don't want memoserv notifications to
go down in channel messages. Our first fix looked like: (I hope there is a better way to do this)
@@ -268,7 +268,7 @@
-            send_message(other, cmd->parent->bot, "MSMSG_NEW_MESSAGE", user->nick, ma->recvd.used - 1);
+            send_message_type(5, other, cmd->parent->bot, user_find_message(other, "MSMSG_NEW_MESSAGE"), user->nick, ma->recvd.used - 1);
And the same in line @@ -563,7 +563,7 @@


Last idea:
If a user sets memoserv in "private" mode with "~set private" then an other user can simply add this user to a channel he owns, now send a message to him, and then delete him again to bypass the "security-checks" of "set private". A fix could be checking for "last-seen" and reject
sending the message if the recipient has never been in the channel.

And the really last idea:
Why not sending the current settings with memoserv.set instead of a list of all subcommands?


I hope I explained it in an understandable way ;)
so long
David Herrmann - gix

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=403001&aid=2668866&group_id=31654