Gadu-Gadu security issues

[Daniel Atallah]

In the interest of making progress toward getting a 2.10.7 out the
door and getting the known security issues resolved in it, here are
some issues that need analysis. Here are the "High Impact" Gadu-Gadu issues

These came from the Coverity static analysis.

Tomasz - are these things that you can look at?  I don't think anyone
else is at all familiar with the gg code.

I'm not sure what the best thing to do in the main-2.x.y branch with
these is going to be since so much has changed in default.
For things that are completely different in default, perhaps a bandaid
type of solution in main-2.x.y would be most appropriate (and also
making sure that the issue doesn't exist in default).

Note that fixes for these shouldn't be pushed to a public repo until
we're about to release.  We need to figure out a better way to handle
that issue.

CID 732074
libpurple/protocols/gg/lib/libgadu.c:528
gg_debug_session(sess, GG_DEBUG_MISC, "// gg_recv_packet() header
recv(%d,%p,%d) = %d\n", sess->fd, &h + sess->header_done, sizeof(h) -
sess->header_done, ret);
 * Illegal address computation (OVERRUN)At (7): "&h +
sess->header_done" evaluates to an address that is at byte offset 56
of an array of 8 bytes.

CID 731944 (This is kind of the same thing as the previous one)
libpurple/protocols/gg/lib/libgadu.c:528
* Out-of-bounds access (ARRAY_VS_SINGLETON)At (7): Using "&h" as an
array. This might corrupt or misinterpret adjacent memory locations.

CID 732164 (I think this is a false positive)
libpurple/protocols/gg/lib/libgadu.c:1187
gg_image_queue_remove(sess, sess->images, 1);
 * Use after free (USE_AFTER_FREE)At (9): Passing freed pointer
"sess->images" as an argument to function
"gg_image_queue_remove(struct gg_session *, struct gg_image_queue *,
int)".
 ** It thinks that because gg_image_queue_remove frees the
"sess->images", the while loop conditional will dereference freed
memory, but I don't think that's actually possible.

CID 731948
libpurple/protocols/gg/lib/dcc7.c:658
strncpy((char*) s.filename, (char*) tmp->filename, GG_DCC7_FILENAME_LEN);
 * Buffer not null terminated (BUFFER_SIZE_WARNING)At (15): Calling
strncpy with a maximum size argument of 255 bytes on destination array
"s.filename" of size 255 bytes might leave the destination string
unterminated.

CID 732124
libpurple/protocols/gg/lib/dcc7.c:252
if (bind(fd, (struct sockaddr*) &sin, sizeof(sin)) == -1) {
 * Uninitialized scalar variable (UNINIT)At (6): Using uninitialized
value "sin": field "sin"."sin_zero" is uninitialized when calling
"bind(int, struct sockaddr const *, socklen_t)".

CID 732123 (This is the same as the last one in a different place)
libpurple/protocols/gg/lib/dcc.c:432
if (!bind(sock, (struct sockaddr*) &sin, sizeof(sin)))

CID 732122 (This is the same as the last one in a different place)
libpurple/protocols/gg/lib/common.c:283
if (connect(sock, (struct sockaddr*) &sin, sizeof(sin)) == -1) {

CID 732073
libpurple/protocols/gg/lib/common.c:117
vsnprintf(buf, size + 1, format, ap);
 * Out-of-bounds access (OVERRUN)At (11): Overrunning dynamic array
"buf" by passing it to a function that indexes it with "size + 1" - 1.
 ** I'm not sure under which conditions this code would be used - it
looks like GG_CONFIG_HAVE_C99_VSNPRINTF must not be defined


There are a number more issues in the "Medium Impact" and "Low Impact"
buckets that I would include if there was a way to export them
non-manually.

For example, the following is interesting:
CID 73200
libpurple/protocols/gg/lib/dcc7.c:149
if (tmp->peer_uin == uin && !tmp->state == GG_STATE_WAITING_FOR_ACCEPT)
* Missing parentheses (CONSTANT_EXPRESSION_RESULT) [select issue]
* At condition "!tmp->state == 39", the value of "tmp->state" must be
between 0 and 1.
* The condition "!tmp->state == 39" cannot be true.

-D