PREfast says "Dereferencing NULL ptr Dest.Buffer

I’m just starting with PREfast. It sounds like it should be a very useful tool. However I don’t quite understand why it’s not happy with the code
below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK. I know that I am null terminating a string
unecessarily, but I don’t think that it’s a bug.
void
TapUnicodeAppend
(
UNICODE_STRING & Dest,
UNICODE_STRING & Src
)
{

if ( !Dest.Buffer )
{

Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, Src.Length + sizeof( WCHAR ), TAP_TAG);
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’;

}
else
{

PWSTR pOldBuffer( NULL );
USHORT CombLen = Dest.Length + Src.Length + sizeof( WCHAR );
if ( CombLen > Dest.MaximumLength )
{
pOldBuffer = Dest.Buffer;
Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, CombLen, TAP_TAG);
if ( pOldBuffer )
{
RtlCopyMemory( Dest.Buffer, pOldBuffer, Dest.Length );
ExFreePool( (PVOID) pOldBuffer );
}
Dest.MaximumLength = CombLen;
}
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’; // PREfast says dereferencing NULL ptr here

}

}

Consider what would happen if ExAllocatePoolWithTag fails to allocate memory.

  • S

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of Michael Wade
Sent: Sunday, January 16, 2011 11:24 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] PREfast says "Dereferencing NULL ptr Dest.Buffer

I’m just starting with PREfast. It sounds like it should be a very useful tool. However I don’t quite understand why it’s not happy with the code below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK. I know that I am null terminating a string unecessarily, but I don’t think that it’s a bug.
void
TapUnicodeAppend
(
UNICODE_STRING & Dest,
UNICODE_STRING & Src
)
{

if ( !Dest.Buffer )
{

Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, Src.Length + sizeof( WCHAR ), TAP_TAG);
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’;

}
else
{

PWSTR pOldBuffer( NULL );
USHORT CombLen = Dest.Length + Src.Length + sizeof( WCHAR );
if ( CombLen > Dest.MaximumLength )
{
pOldBuffer = Dest.Buffer;
Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, CombLen, TAP_TAG);
if ( pOldBuffer )
{
RtlCopyMemory( Dest.Buffer, pOldBuffer, Dest.Length );
ExFreePool( (PVOID) pOldBuffer );
}
Dest.MaximumLength = CombLen;
}
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’; // PREfast says dereferencing NULL ptr here

}

}


NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer

Apologies for the second reply.

The code you have written is also somewhat unsafe in that Dest.Length + Src.Length + sizeof(WCHAR) could overflow the capacity of USHORT (remember also that the UNICODE_STRING encodes its lengths as USHORTs). If either Dest or Src is a non-static string (i.e. comes from the user or the like), then you’ll want to write code to handle this appropriately for your situation.

  • S

-----Original Message-----
From: xxxxx@lists.osr.com [mailto:xxxxx@lists.osr.com] On Behalf Of Michael Wade
Sent: Sunday, January 16, 2011 11:24 AM
To: Windows System Software Devs Interest List
Subject: [ntdev] PREfast says "Dereferencing NULL ptr Dest.Buffer

I’m just starting with PREfast. It sounds like it should be a very useful tool. However I don’t quite understand why it’s not happy with the code below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK. I know that I am null terminating a string unecessarily, but I don’t think that it’s a bug.
void
TapUnicodeAppend
(
UNICODE_STRING & Dest,
UNICODE_STRING & Src
)
{

if ( !Dest.Buffer )
{

Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, Src.Length + sizeof( WCHAR ), TAP_TAG);
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’;

}
else
{

PWSTR pOldBuffer( NULL );
USHORT CombLen = Dest.Length + Src.Length + sizeof( WCHAR );
if ( CombLen > Dest.MaximumLength )
{
pOldBuffer = Dest.Buffer;
Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, CombLen, TAP_TAG);
if ( pOldBuffer )
{
RtlCopyMemory( Dest.Buffer, pOldBuffer, Dest.Length );
ExFreePool( (PVOID) pOldBuffer );
}
Dest.MaximumLength = CombLen;
}
RtlAppendUnicodeStringToString( &Dest, &Src );
Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’; // PREfast says dereferencing NULL ptr here

}

}


NTDEV is sponsored by OSR

For our schedule of WDF, WDM, debugging and other seminars visit:
http://www.osr.com/seminars

To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer

In addition to Sky’s comment, take a look at kernel function prototypes that
manipulate strings. Like RtlUnicodeStringCat. They typically use
PUNICODE_STRING as arguments.

Thomas F. Divine
http://www.pcausa.com


From: “Michael Wade”
Sent: Sunday, January 16, 2011 2:24 PM
Newsgroups: ntdev
To: “Windows System Software Devs Interest List”
Subject: [ntdev] PREfast says "Dereferencing NULL ptr Dest.Buffer

> I’m just starting with PREfast. It sounds like it should be a very useful
> tool. However I don’t quite understand why it’s not happy with the code
> below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I
> think I am making sure it’s OK. I know that I am null terminating a
> string
> unecessarily, but I don’t think that it’s a bug.
> void
> TapUnicodeAppend
> (
> UNICODE_STRING & Dest,
> UNICODE_STRING & Src
> )
> {
>
> if ( !Dest.Buffer )
> {
>
> Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, Src.Length
> + sizeof( WCHAR ), TAP_TAG);
> RtlAppendUnicodeStringToString( &Dest, &Src );
> Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’;
>
> }
> else
> {
>
> PWSTR pOldBuffer( NULL );
> USHORT CombLen = Dest.Length + Src.Length + sizeof( WCHAR );
> if ( CombLen > Dest.MaximumLength )
> {
> pOldBuffer = Dest.Buffer;
> Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool,
> CombLen, TAP_TAG);
> if ( pOldBuffer )
> {
> RtlCopyMemory( Dest.Buffer, pOldBuffer, Dest.Length );
> ExFreePool( (PVOID) pOldBuffer );
> }
> Dest.MaximumLength = CombLen;
> }
> RtlAppendUnicodeStringToString( &Dest, &Src );
> Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’; // PREfast says
> dereferencing NULL ptr here
>
> }
>
> }
>
> —
> NTDEV is sponsored by OSR
>
> For our schedule of WDF, WDM, debugging and other seminars visit:
> http://www.osr.com/seminars
>
> To unsubscribe, visit the List Server section of OSR Online at
> http://www.osronline.com/page.cfm?name=ListServer

Michael Wade wrote:

I’m just starting with PREfast. It sounds like it should be a very useful tool. However I don’t quite understand why it’s not happy with the code below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK.

Where do you think you are doing that? You allocate memory for
Dest.Buffer twice in this code, and in neither instance do you check
whether the call succeeded.


Tim Roberts, xxxxx@probo.com
Providenza & Boekelheide, Inc.

Long time since I used prefast and I do not have the DDK installed on
this machine but doesn’t prefast actually show the path that leads to
the error?

/Faik

On Mon, Jan 17, 2011 at 7:19 PM, Tim Roberts wrote:
> Michael Wade wrote:
>> I’m just starting with PREfast. ?It sounds like it should be a very useful tool. ?However I don’t quite understand why it’s not happy with the code below. ?It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK.
>
> Where do you think you are doing that? ?You allocate memory for
> Dest.Buffer twice in this code, and in neither instance do you check
> whether the call succeeded.
>
> –
> Tim Roberts, xxxxx@probo.com
> Providenza & Boekelheide, Inc.
>
>
> —
> NTDEV is sponsored by OSR
>
> For our schedule of WDF, WDM, debugging and other seminars visit:
> http://www.osr.com/seminars
>
> To unsubscribe, visit the List Server section of OSR Online at http://www.osronline.com/page.cfm?name=ListServer
>

You do not check ExAllocatePoolWithTag for failure.


Maxim S. Shatskih
Windows DDK MVP
xxxxx@storagecraft.com
http://www.storagecraft.com

“Michael Wade” wrote in message news:xxxxx@ntdev…
> I’m just starting with PREfast. It sounds like it should be a very useful tool. However I don’t quite understand why it’s not happy with the code
> below. It says that I am dereferencing a NULL ptr in Dest.Buffer but I think I am making sure it’s OK. I know that I am null terminating a string
> unecessarily, but I don’t think that it’s a bug.
> void
> TapUnicodeAppend
> (
> UNICODE_STRING & Dest,
> UNICODE_STRING & Src
> )
> {
>
> if ( !Dest.Buffer )
> {
>
> Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, Src.Length + sizeof( WCHAR ), TAP_TAG);
> RtlAppendUnicodeStringToString( &Dest, &Src );
> Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’;
>
> }
> else
> {
>
> PWSTR pOldBuffer( NULL );
> USHORT CombLen = Dest.Length + Src.Length + sizeof( WCHAR );
> if ( CombLen > Dest.MaximumLength )
> {
> pOldBuffer = Dest.Buffer;
> Dest.Buffer = (PWSTR) ExAllocatePoolWithTag( NonPagedPool, CombLen, TAP_TAG);
> if ( pOldBuffer )
> {
> RtlCopyMemory( Dest.Buffer, pOldBuffer, Dest.Length );
> ExFreePool( (PVOID) pOldBuffer );
> }
> Dest.MaximumLength = CombLen;
> }
> RtlAppendUnicodeStringToString( &Dest, &Src );
> Dest.Buffer[Dest.Length/sizeof(WCHAR)] = ‘\0’; // PREfast says dereferencing NULL ptr here
>
> }
>
> }
>