[ros-dev] [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.

[Magnus Johnsson]

Eric, the thing is, buffer overflows don't just crash the program unless
you have some really nifty guard pages, but overwrite other things in
memory. This means an attacker can, in certain situations, use it to create
something that not just crashes, but with a nifty input create an exploit.
Having a 'Oh it will just crash' attitude is 'not the best'.

2018-04-02 19:41 GMT+02:00 Eric Kohl <eric.kohl at t-online.de>:

> 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
> > Ros-dev at reactos.org
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
>
> _______________________________________________
> Ros-dev mailing list
> Ros-dev at reactos.org
> http://www.reactos.org/mailman/listinfo/ros-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20180402/4acc89d9/attachment-0001.html>