Mercurial > pidgin
comparison libpurple/protocols/qq/qq_network.c @ 28213:33921125348d
The output buffer passed to qq_encrypt needs to be 17 bytes bigger
than the data you're encrypting, not 16 bytes bigger. Fixes #10191.
It's hard to say whether this actually causes problems. My guess is
that it does not.
However, the way the qq protocol plugin constructs the plain text
buffer to be passed to qq_encrypt is error prone, and the many calls
to g_newa(guint8, MAX_PACKET_SIZE) are really bad because
MAX_PACKET_SIZE is 64KB. This is a ridiculous amount of space to
request on the stack. All these qq_put8 qq_put16 qq_put32 qq_putdata
functions should be changed to insert data into a dynamically
allocated GString instead of the stack-allocated buffers that they
use now. This eliminates the potential for accidentally overwriting
the end of the buffer.
And the second g_newa() for the output buffer passed into qq_encrypt()
should be changed to allocate space on the heap in most places because,
as previously noted, 64KB is a ridiculous amount of memory to request
from the stack.
Heap allocation may be expensive when compared to stack allocation, but
I feel it's usually worth it to eliminate the possibilty of buffer
overflow.
author | Mark Doliner <mark@kingant.net> |
---|---|
date | Fri, 04 Sep 2009 22:50:26 +0000 |
parents | f541583e31bd |
children | 9ab9e5f1eec2 |
comparison
equal
deleted
inserted
replaced
28210:9af95186dde4 | 28213:33921125348d |
---|---|
1144 | 1144 |
1145 g_return_val_if_fail(gc != NULL && gc->proto_data != NULL, -1); | 1145 g_return_val_if_fail(gc != NULL && gc->proto_data != NULL, -1); |
1146 qd = (qq_data *)gc->proto_data; | 1146 qd = (qq_data *)gc->proto_data; |
1147 g_return_val_if_fail(data != NULL && data_len > 0, -1); | 1147 g_return_val_if_fail(data != NULL && data_len > 0, -1); |
1148 | 1148 |
1149 /* at most 16 bytes more */ | 1149 /* at most 17 bytes more */ |
1150 encrypted = g_newa(guint8, data_len + 16); | 1150 encrypted = g_newa(guint8, data_len + 17); |
1151 encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key); | 1151 encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key); |
1152 if (encrypted_len < 16) { | 1152 if (encrypted_len < 16) { |
1153 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] 0x%04X %s\n", | 1153 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] 0x%04X %s\n", |
1154 encrypted_len, seq, cmd, qq_get_cmd_desc(cmd)); | 1154 encrypted_len, seq, cmd, qq_get_cmd_desc(cmd)); |
1155 return -1; | 1155 return -1; |
1221 | 1221 |
1222 #if 1 | 1222 #if 1 |
1223 purple_debug_info("QQ", "<== [SRV-%05d] %s(0x%04X), datalen %d\n", | 1223 purple_debug_info("QQ", "<== [SRV-%05d] %s(0x%04X), datalen %d\n", |
1224 seq, qq_get_cmd_desc(cmd), cmd, data_len); | 1224 seq, qq_get_cmd_desc(cmd), cmd, data_len); |
1225 #endif | 1225 #endif |
1226 /* at most 16 bytes more */ | 1226 /* at most 17 bytes more */ |
1227 encrypted = g_newa(guint8, data_len + 16); | 1227 encrypted = g_newa(guint8, data_len + 17); |
1228 encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key); | 1228 encrypted_len = qq_encrypt(encrypted, data, data_len, qd->session_key); |
1229 if (encrypted_len < 16) { | 1229 if (encrypted_len < 16) { |
1230 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] 0x%04X %s\n", | 1230 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] 0x%04X %s\n", |
1231 encrypted_len, seq, cmd, qq_get_cmd_desc(cmd)); | 1231 encrypted_len, seq, cmd, qq_get_cmd_desc(cmd)); |
1232 return -1; | 1232 return -1; |
1268 | 1268 |
1269 qd->send_seq++; | 1269 qd->send_seq++; |
1270 seq = qd->send_seq; | 1270 seq = qd->send_seq; |
1271 | 1271 |
1272 /* Encrypt to encrypted with session_key */ | 1272 /* Encrypt to encrypted with session_key */ |
1273 /* at most 16 bytes more */ | 1273 /* at most 17 bytes more */ |
1274 encrypted = g_newa(guint8, buf_len + 16); | 1274 encrypted = g_newa(guint8, buf_len + 17); |
1275 encrypted_len = qq_encrypt(encrypted, buf, buf_len, qd->session_key); | 1275 encrypted_len = qq_encrypt(encrypted, buf, buf_len, qd->session_key); |
1276 if (encrypted_len < 16) { | 1276 if (encrypted_len < 16) { |
1277 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] %s (0x%02X)\n", | 1277 purple_debug_error("QQ_ENCRYPT", "Error len %d: [%05d] %s (0x%02X)\n", |
1278 encrypted_len, seq, qq_get_room_cmd_desc(room_cmd), room_cmd); | 1278 encrypted_len, seq, qq_get_room_cmd_desc(room_cmd), room_cmd); |
1279 return -1; | 1279 return -1; |