[VIM] Provable vendor ACK in csDoom

[Steven M. Christey]

OK, so when Luigi Auriemma says an issue is vendor-acknowledged we all
have reasonable cause to believe him, but I went the extra mile to
prove it.

Ref: http://aluigi.altervista.org/adv/csdoombof-adv.txt

Claimed vendor acknowledgement and patch.

The vendor home page here:

  http://voxelsoft.com/csdoom/

has undated and uncredited statements under the "Features" section:

  Added:
    * Multiple buffer overflow fixes
    * Multiple string handling fixes

The "Latest Sources" section had source code updated 25/03/2006,
another correlator.

Still - historically, CVE has not treated discloser-claimed
acknowledgement as true vendor acknowledgement, even for reliable
researchers (though I'm reconsidering that policy 'cause sometimes it
means we spend a lot of time trying to prove what we're already 95%
confident about).  Anyway, I dug a bit further.

Grabbing the source code from the vendor site and grepping for "luigi"
gave me a mild surprise:

In ./server/c_console.cpp:

>int PrintString (int printlevel, char *outline)
>{
>        static bool last_string_terminated = true;
>        char szBuffer[9000]; // thanks to Luigi Auriemma for finding
>an overflow bug here

OK so the vendor doesn't make this sound exactly like a format string,
but the source code no longer has this piece of code, as quoted in
Auriemma's original advisory:

  printf (outline);

All sprintf/printf statements in PrintString() now have constant
format strings.

Similarly, in server/sv_main.cpp we now have:

>void STACK_ARGS SV_BroadcastPrintf (int level, const char *fmt, ...)
>{
>    va_list       argptr;
>    char          string[4096]; // thanks to Luigi Auriemma for finding an overflow bug here
>

but notice this change as well:

>void STACK_ARGS SV_BroadcastPrintfExcluding (int level, int client_nosend, const char *fmt, ...)
>{
>    va_list       argptr;
>    char          string[4096]; // thanks to Luigi Auriemma for finding an overflow bug here


Auriemma did not mention SV_BroadcastPrintfExcluding() in his
advisory.


While uncredited, there do appear to be relevant changes to
SV_SetupUserInfo() in sv_main.cpp:

>    strcpy(string, MSG_ReadString());// changed
>		if(/*!strlen(string) ||*/ strlen(string) > MAXPLAYERNAME || strstr(string, "%"))
>
>...
>
>	strcpy(p->userinfo.netname, string);

... which looks like a sanity check against string length for the
strcpy() function call.

Similarly, you have:

>    strcpy(string, MSG_ReadString());
>		if(strlen(string) > MAXPLAYERNAME || strstr(string, "%"))
>
>...
>		strcpy(p->userinfo.team, string); // changed

... which is also a sanity check.


Thus, as originally claimed by the researcher, the vendor has
acknowledged the issue via a vague changelog and explicit patches.

- Steve