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 : ros-dev(a)reactos.org
>  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
>  Ros-dev(a)reactos.org
>  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 : ros-dev(a)reactos.org
> >  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
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev 
>  
>  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>