RE: [NeoStats-Devel] [Commits] r2797 - in trunk: . include modules/connectserv src

"M" <[email protected]> Sat, 10 Sep 2005 00:13:52 +0100
Newsgroups gmane.comp.neostats.devel
Message-ID <[email protected]>
> Author: Fish
> escape any % format specifiers we recieve from clients

I am guessing you are "protecting" against some potential rogue format
specifier in the real name field since % is not a valid character for the
majority of user fields. 

Any check of this string should be done at the point of entry. Modules
should be able to trust any and all data they receive from the core so a
simple core check of the initial string would be far better than making
modules process any data they send and receive in this way. 

In this particular instance, changing:

irc_chanalert( cs_bot, buf ) to irc_chanalert( cs_bot, "%s", buf )

and 

nlog( LOG_NORMAL, buf ) to nlog( LOG_NORMAL, "%s", buf )

would have been just as effective in preventing rogue format specifiers in
the whole of the connectserv reporting sequence and have far less CPU cost
than parsing an entire buffer for every single report regardless of the
opportunity for problems.

Since connectserv is merely combining strings received from the core, they
should be considered trusted making nlog( LOG_NORMAL, buf ) a valid
operation without any string "cleaning" required. Any string cleaning should
be handled by the core before a module gets the string.

Mark.