Re: [ros-dev] [ros-diffs] [reactos] 01/01: CID 1206831 Dereference after null check

[Jerome Gardou]

HI,

that doesn't look good, as shown by 
https://reactos.org/testman/compare.php?ids=56275,56276

Jérôme

Le 29/10/2017 à 11:17, Samuel Serapion a écrit :
> https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b3b2a23f05e5188dc14759...
>
> commit b3b2a23f05e5188dc1475961fcd7f036f0046d25
> Author: Samuel Serapion [e-mail]
> AuthorDate: Fri Oct 20 14:00:32 2017 -0400
>  CID 1206831 Dereference after null check
>
>  BytesRead is an optional out parameter and must be checked before being written to.
>
> sdk/lib/rtl/memstream.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/sdk/lib/rtl/memstream.c b/sdk/lib/rtl/memstream.c
> index 0549424ca4..8fe4169fb1 100644
> --- a/sdk/lib/rtl/memstream.c
> +++ b/sdk/lib/rtl/memstream.c
> @@ -185,7 +185,8 @@ RtlReadMemoryStream(
>   Stream->Current = (PUCHAR)Stream->Current + CopyLength;
>
> *BytesRead = CopyLength;
>
> if (BytesRead)
>    *BytesRead = CopyLength;
>
> return S_OK;
>
> }

[encoded]

Seems like this API has a 'feature' where by it throws exceptions if BytesRead
is null?

On Sun, Oct 29, 2017 at 8:02 AM, Jerome Gardou [e-mail]
wrote:
> HI,
>
> that doesn't look good, as shown by https://reactos.org/testman/co
> mpare.php?ids=56275,56276
>
> Jérôme
>
> Le 29/10/2017 à 11:17, Samuel Serapion a écrit :
> > https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b3b2a2
> > 3f05e5188dc1475961fcd7f036f0046d25
> >
> > commit b3b2a23f05e5188dc1475961fcd7f036f0046d25
> > Author: Samuel Serapion [e-mail]
> > AuthorDate: Fri Oct 20 14:00:32 2017 -0400
> >  CID 1206831 Dereference after null check
> >       BytesRead is an optional out parameter and must be checked
> > before being written to.
> >
> > sdk/lib/rtl/memstream.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/sdk/lib/rtl/memstream.c b/sdk/lib/rtl/memstream.c
> > index 0549424ca4..8fe4169fb1 100644
> > --- a/sdk/lib/rtl/memstream.c
> > +++ b/sdk/lib/rtl/memstream.c
> > @@ -185,7 +185,8 @@ RtlReadMemoryStream(
> >         Stream->Current = (PUCHAR)Stream->Current + CopyLength;
> > *BytesRead = CopyLength;
> > if (BytesRead)
> >    *BytesRead = CopyLength;
> >   return S_OK;
> > }
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Thomas Faber]

Or the annotation is just wrong and this parameter is not optional?

On 2017-10-31 16:07, encoded wrote:
> Seems like this API has a 'feature' where by it throws exceptions if BytesRead
> is null?
>
> On Sun, Oct 29, 2017 at 8:02 AM, Jerome Gardou [e-mail]
> wrote:
> > HI,
> >
> > that doesn't look good, as shown by https://reactos.org/testman/co
> > mpare.php?ids=56275,56276
> >
> > Jérôme
> >
> > Le 29/10/2017 à 11:17, Samuel Serapion a écrit :
> > > https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b3b2a2
> > > 3f05e5188dc1475961fcd7f036f0046d25
> > >
> > > commit b3b2a23f05e5188dc1475961fcd7f036f0046d25
> > > Author: Samuel Serapion [e-mail]
> > > AuthorDate: Fri Oct 20 14:00:32 2017 -0400
> > >   CID 1206831 Dereference after null check
> > >        BytesRead is an optional out parameter and must be checked
> > > before being written to.
> > >
> > > sdk/lib/rtl/memstream.c | 3 ++-
> > >    1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/sdk/lib/rtl/memstream.c b/sdk/lib/rtl/memstream.c
> > > index 0549424ca4..8fe4169fb1 100644
> > > --- a/sdk/lib/rtl/memstream.c
> > > +++ b/sdk/lib/rtl/memstream.c
> > > @@ -185,7 +185,8 @@ RtlReadMemoryStream(
> > >          Stream->Current = (PUCHAR)Stream->Current + CopyLength;
> > > *BytesRead = CopyLength;
> > > if (BytesRead)
> > >    *BytesRead = CopyLength;
> > >    return S_OK;
> > > }