[IRCServices] what do they think ?

[Andrew Church]

>i had wrote this function on my services, it is working well, but i wold like
>to hear coments and sugestions on it
>
>it send password information to the nick email (ni->email) and i changed the
>do_register, so users must give a mail

     From a functional standpoint, it's something I'd planned to do for quite
a while.  From an implementation standpoint, I'd hang myself before releasing
code of this quality.  No offense--if it works for you, fine; but there are
numerous potential problems and security holes in this implementation.  To
point out a few: (further comments at the end of the message)

>      lock = fopen(".senpass.nick", "r");

     (1) There's a huge distance between this check and the time the file
is created, leaving a big window for race conditions (except that since
you process this all in the same thread you don't need a lock in the first
place).

     (2) Doing all the processing in the same thread, and especially waiting
for sendmail to finish, will slow down Services immensely.  (Suppose your
sendmail is set to verify recipients' domain names before queueing mail; what
happens if someone registers a nick with an address in a domain they own, and
then shut off the nameserver for that domain?)

     (3) This filename has a typo and so locking wouldn't ever work anyway.

>    } else if (!strchr(ni->email, '@')) {
[...]
>    } else if(strlen(ni->email)>50) {

     This is a pretty weak check on E-mail address syntax.

>       strcpy(illsend,"/usr/sbin/sendmail ");
>       strcat(illsend, ni->email);
>       strcat(illsend," < .sendpass.nick");
[...]
>          system(illsend);

     Lovely.  Suppose I set my E-mail address to "foo@;IFS=.;rm.-rf./"?
Poof, there goes your system (or whatever part of it Services can access--
I hope you're not running as root).  Not to mention the problems you get
with having a bogus "sendmail" in a directory in your PATH.

     For the record, the proper way to do this would be to open a direct SMTP
connection to a known mail server and send the mail over that connection,
using select() to monitor the status of the connection (and timing out in a
reasonable period of time to prevent people from using up all file
descriptors by sending lots of SENDPASS requests).  Note that a proper
implementation of this functionality requires quite a bit of work, including
redoing the main program loop and I/O code to be able to monitor multiple
sockets at once; this is why I never got around to adding this functionality.
If it were as simple as writing a function like the original poster did, I
would have done it long ago.

     Andrew (Kempe):  I'm willing to help you with this if you want, but you
need to be very careful about adding functionality like this; it can turn
into Swiss cheese (security- and stability-wise) if you don't watch out.

  --Andrew Church
    achurch at dragonfire.net
    http://achurch.dragonfire.net/
---------------------------------------------------------------
To unsubscribe, send email to majordomo at ender.shadowfire.org
with "unsubscribe ircservices" in the body, without the quotes.