#2 PVS-Studio

[Andrey Karpov]

Hello, All.

I checked the UCSniff project using the PVS-Studio static code analyzer.
I just glanced through the code but managed to find a few obviously odd
fragments. Below I will cite the analyzer-generated messages I have
studied and the corresponding code fragments. I hope this will help to
improve the project a bit.

You may review other odd fragments by downloading PVS-Studio from here:
http://www.viva64.com/en/pvs-studio-download/

I can also give you a registration key for some time.
You are welcome to ask questions here:
http://www.viva64.com/en/about-feedback/

--
Andrey Karpov, MVP
Cand. Sc. (Physics and Mathematics), CTO
OOO "Program Verification Systems" (Co Ltd)
URL: www.viva64.com
E-Mail: karpov@viva64.com

=======================================
V512 A call of the 'memcpy' function will lead to the '"sip"' buffer becoming out of range. targets.c 566

struct targets{
char ip[MAX_ASCII_ADDR_LEN];
u_char mac[MAX_ASCII_ADDR_LEN];
char extension[64];
char dirname[64];
char protocol[11];
char ua[48];
char misc[64];
};

void sip_targetlookup(sipDB* currentSipCall)
{
...
memcpy(targettab[targetcount].protocol,"sip",sizeof(targettab[targetcount].protocol));
...
}

Additional:
V512 A call of the 'memcpy' function will lead to the '"sip"' buffer becoming out of range. targets.c 631
=======================================
V523 The 'then' statement is equivalent to the 'else' statement. ec_sccp.c 3216

int openreceivehandler(....)
{
...
int return_rtp = check_rtp_entry(ip);
if (return_rtp < 0) {
int return_value = populate_rtp_struct(return_rtp, ip, passthrupartyid, rtpport);
if(return_value == -1) {
return NULL;
}
} else {
int return_value = populate_rtp_struct(return_rtp, ip, passthrupartyid, rtpport);
if (return_value == -1) {
return NULL;
}
}
...
}

Additional:
V523 The 'then' statement is equivalent to the 'else' statement. ec_sccp.c 3794
=======================================
V541 It is dangerous to print the string 'dst' into itself. vc_format.c 177

int hex_format(...., u_char *dst)
{
...
sprintf(dst, "%s %04x: ", dst, i );
...
}

Additional:
V541 It is dangerous to print the string 'dst' into itself. vc_format.c 183
V541 It is dangerous to print the string 'dst' into itself. vc_format.c 185
V541 It is dangerous to print the string 'dst' into itself. vc_format.c 200
=======================================
V547 Expression '* hDecoder < 0' is always false. Unsigned type value is never < 0. g726_decoder.c 11

int initialize_g726_decoder(unsigned long *hDecoder)
{
*hDecoder = EasyG726_init_decoder();
if(*hDecoder<0)
return -1;
return 0;
}

int initialize_g726_decoder(unsigned long *hDecoder)
{
*hDecoder = EasyG726_init_decoder();
if(*hDecoder<0)
return -1;
return 0;
}

Additional:
V547 Expression 'currentRTPCall->hDecoderFwd >= 0' is always true. Unsigned type value is always >= 0. ec_siprtp.c 1547
V547 Expression 'currentRTPCall->hDecoderRev >= 0' is always true. Unsigned type value is always >= 0. ec_siprtp.c 1550
=======================================
V579 The memset function receives the pointer and its size as arguments. It is possibly a mistake. Inspect the third argument. directory_parser.c 1338

int check_name_value(... ,char *target)
{
...
memset(target,'\0',sizeof(target));
...
}
=======================================
V597 The compiler could delete the 'memset' function call, which is used to flush 'send_msg' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. http_handler.c 84

int request_http_corporate_url(char *directory_buf,int message_id)
{
char send_msg[2048] = {'\0'};
...
memset(send_msg,'\0',sizeof(send_msg));
...
}

Additional:
V597 The compiler could delete the 'memset' function call, which is used to flush 'name_start_tag' buffer. The RtlSecureZeroMemory() function should be used to erase the private data. directory_parser.c 941
=======================================
V610 Undefined behavior. Check the shift operator '<<. The left operand '~1' is negative. ec_ip.c 187

#define FP_HOST_NONLOCAL 1<<1

FUNC_DECODER(decode_ip)
{
...
PACKET->PASSIVE.flags &= ~FP_HOST_NONLOCAL;
...
}

Additional:
V610 Undefined behavior. Check the shift operator '<<. The left operand '~1' is negative. ec_ip6.c 110
=======================================