Re: [ros-diffs] 01/01: [WINLOGON] Clean up part 2 - Replace the UNICODE_STRING usMessage by a PWSTR pszMessage. - Use the "%02d:%02d:%02d" time format and get rid of the safe string printf because the string will NEVER be longer than 8 characters. - Rename the timer id constant because it is NOT a resource id. - Rename variables according to the coding style.

Hello Thomas,

you're right, using the run-time size checks are a good way to keep
application from crashing because of buffer overflows. They'll just keep
on using corrupt data instead! If you want to fix this problem: Don't
use C! Use C++, C#, Java etc. instead!

I prefer to see an application crash because of a buffer overflow rather
than seeing it store truncated phone numbers in a database.

PS: If the timeout is longer than a day, winlogon uses the "%d days"
format. In the end, a buffer of 10 characters is still large enough.

PPS: I'll keep using the old functions until you remove them from the
runtime code.


Regards
Eric

Am 02.04.2018 um 14:12 schrieb Thomas Faber:
> Hey Eric,
> 
> On 2018-04-02 12:58, Eric Kohl wrote:
>> -    RtlStringCbPrintfW(strbuf, sizeof(strbuf), L"%d:%d:%d", hours,
>> minutes, seconds);
>> +    swprintf(szBuffer, L"%02d:%02d:%02d", iHours, iMinutes, iSeconds);
> 
> Unfortunately I must disagree with this change.
> 
> Buffer overflows are a big enough threat that code review and
> static analysis are not generally considered sufficient to protect
> against them.
> So it's best practice for new code to always verify sizes at run-time,
> and never to use s(w)print.
> 
> Best regards,
> Thomas
> 
> PS: from what I see, iHours can be as large as 1193046, which won't
>     fit in 2 digits
> 
> _______________________________________________
> Ros-dev mailing list
> [email protected]
> http://www.reactos.org/mailman/listinfo/ros-dev


_______________________________________________
Ros-dev mailing list
[email protected]
http://www.reactos.org/mailman/listinfo/ros-dev