Originally posted by [b]theultramage[/b]
http://www.eathena.ws/board/index.php?autocom=bugtracker&showbug=3080
The code still uses strncpy() in multiple places. The spec says
QUOTE
The strncpy() function shall copy not more than n bytes (bytes that follow a null byte are not copied) from the array pointed to by s2 to the array pointed to by s1. If the array pointed to by s2 is a string that is shorter than n bytes, null bytes shall be appended to the copy in the array pointed to by s1, until n bytes in all are written. If there is no null byte in the first n bytes of the array pointed to by s2, the result is not null-terminated.
In practice, it means when you do "strncpy(str, "123456789012345678901234", NAME_LENGTH)", the result will be a string with no zero terminator. Reads on this string will scan past the end of the buffer, and naive strcpy's from this buffer will cause memory corruption.
Even worse are uneducated attempts at "strncpy(str, "12345678901234567890123", NAME_LENGTH-1)", which are guaranteed to produce an unterminated string even on perfectly valid inputs.
However, there is some code that explicitly requires that the produced buffer be copied whole and without a terminating zero. This appears to apply to some server->client packets.
We have implemented safestrncpy() as an alternative to strncpy() that guarantees termination. All uses of strncpy() should be replaced with safestrncpy() wherever applicable, and places where it is not applicable should be clearly marked as such.