Re: [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 character

[Hermès BÉLUSCA-MAÏTO]

If I remember correctly you can make shutdowns delayed of many days on Windows (using the InitiateSystemShutdown(Ex) function), in which case the 2-digit hour won't work at all.

Best,
Hermès
> -----Message d'origine-----
> De : Ros-dev [mailto:[e-mail]] De la part de Thomas
> Faber
> Envoyé : lundi 2 avril 2018 14:13
> À : Eric Kohl
> Cc : [e-mail]
> Objet : Re: [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 character
>
> 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
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Eric Kohl]

Hello Hermès,

you can use a maximum timeout value of 31536999 seconds (that's 1 second
shy of 10 years). If the timeout value is larger or equal 1 day (>=86400
seconds), winlogon will use the "%d days" format. It will just show a
maximum of "3649 days". No buffer overflow here! :-)

Regards
Eric

Am 02.04.2018 um 14:43 schrieb Hermès BÉLUSCA-MAÏTO:
> If I remember correctly you can make shutdowns delayed of many days on Windows (using the InitiateSystemShutdown(Ex) function), in which case the 2-digit hour won't work at all.
>
> Best,
> Hermès
> > -----Message d'origine-----
> > De : Ros-dev [mailto:[e-mail]] De la part de Thomas
> > Faber
> > Envoyé : lundi 2 avril 2018 14:13
> > À : Eric Kohl
> > Cc : [e-mail]
> > Objet : Re: [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 character
> >
> > 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
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev