Skip to content

Commit b0af190

Browse files
EmmanuelPEmmanuelP
committed
gvcp: avoid out of bound memory access by checking packet size
1 parent 4f63bd1 commit b0af190

File tree

4 files changed

+163
-133
lines changed

4 files changed

+163
-133
lines changed

‎src/arvgvcpprivate.h

+46-34
Original file line numberDiff line numberDiff line change
@@ -357,9 +357,9 @@ void arv_gvcp_packet_debug (const ArvGvcpPacket *packet, ArvDebugLevel lev
357357
*/
358358

359359
static inline ArvGvcpPacketType
360-
arv_gvcp_packet_get_packet_type (ArvGvcpPacket *packet)
360+
arv_gvcp_packet_get_packet_type (ArvGvcpPacket *packet, size_t size)
361361
{
362-
if (packet == NULL)
362+
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
363363
return ARV_GVCP_PACKET_TYPE_ERROR;
364364

365365
return (ArvGvcpPacketType) packet->header.packet_type;
@@ -373,9 +373,9 @@ arv_gvcp_packet_get_packet_type (ArvGvcpPacket *packet)
373373
*/
374374

375375
static inline guint8
376-
arv_gvcp_packet_get_packet_flags (ArvGvcpPacket *packet)
376+
arv_gvcp_packet_get_packet_flags (ArvGvcpPacket *packet, size_t size)
377377
{
378-
if (packet == NULL)
378+
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
379379
return 0;
380380

381381
return (ArvGvcpPacketType) packet->header.packet_flags;
@@ -389,40 +389,42 @@ arv_gvcp_packet_get_packet_flags (ArvGvcpPacket *packet)
389389
*/
390390

391391
static inline ArvGvcpCommand
392-
arv_gvcp_packet_get_command (ArvGvcpPacket *packet)
392+
arv_gvcp_packet_get_command (ArvGvcpPacket *packet, size_t size)
393393
{
394-
if (packet == NULL)
394+
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
395395
return (ArvGvcpCommand) 0;
396396

397397
return (ArvGvcpCommand) g_ntohs (packet->header.command);
398398
}
399399

400400
static inline void
401-
arv_gvcp_packet_set_packet_id (ArvGvcpPacket *packet, guint16 id)
401+
arv_gvcp_packet_set_packet_id (ArvGvcpPacket *packet, guint16 id, size_t size)
402402
{
403-
if (packet != NULL)
403+
if G_UNLIKELY(packet != NULL && size >= sizeof (ArvGvcpPacket))
404404
packet->header.id = g_htons (id);
405405
}
406406

407407
static inline guint16
408-
arv_gvcp_packet_get_packet_id (ArvGvcpPacket *packet)
408+
arv_gvcp_packet_get_packet_id (ArvGvcpPacket *packet, size_t size)
409409
{
410-
if (packet == NULL)
410+
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
411411
return 0;
412412

413413
return g_ntohs (packet->header.id);
414414
}
415415

416416
static inline void
417-
arv_gvcp_packet_get_read_memory_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *size)
417+
arv_gvcp_packet_get_read_memory_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
418+
guint32 *address, guint32 *size)
418419
{
419-
if (packet == NULL) {
420+
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + 2 * sizeof (guint32)) {
420421
if (address != NULL)
421422
*address = 0;
422423
if (size != NULL)
423424
*size = 0;
424425
return;
425426
}
427+
426428
if (address != NULL)
427429
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
428430
if (size != NULL)
@@ -443,69 +445,76 @@ arv_gvcp_packet_get_read_memory_ack_data (const ArvGvcpPacket *packet)
443445
}
444446

445447
static inline void
446-
arv_gvcp_packet_get_write_memory_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *size)
448+
arv_gvcp_packet_get_write_memory_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
449+
guint32 *address, guint32 *size)
447450
{
448-
if (packet == NULL) {
451+
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32)) {
449452
if (address != NULL)
450453
*address = 0;
451454
if (size != NULL)
452455
*size = 0;
453456
return;
454457
}
458+
455459
if (address != NULL)
456460
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
457461
if (size != NULL)
458462
*size = g_ntohs (packet->header.size) - sizeof (guint32);
459463
}
460464

461-
static inline void *
462-
arv_gvcp_packet_get_write_memory_cmd_data (const ArvGvcpPacket *packet)
463-
{
464-
return (char *) packet + sizeof (ArvGvcpPacket) + sizeof (guint32);
465-
}
466-
467465
static inline size_t
468466
arv_gvcp_packet_get_write_memory_ack_size (void)
469467
{
470468
return sizeof (ArvGvcpPacket) + sizeof (guint32);
471469
}
472470

471+
static inline void *
472+
arv_gvcp_packet_get_write_memory_cmd_data (const ArvGvcpPacket *packet)
473+
{
474+
return (char *) packet + sizeof (ArvGvcpPacket) + sizeof (guint32);
475+
}
476+
473477
static inline void
474-
arv_gvcp_packet_get_read_register_cmd_infos (const ArvGvcpPacket *packet, guint32 *address)
478+
arv_gvcp_packet_get_read_register_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
479+
guint32 *address)
475480
{
476-
if (packet == NULL) {
481+
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32)) {
477482
if (address != NULL)
478483
*address = 0;
479484
return;
480485
}
486+
481487
if (address != NULL)
482488
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
483489
}
484490

485-
static inline guint32
486-
arv_gvcp_packet_get_read_register_ack_value (const ArvGvcpPacket *packet)
487-
{
488-
if (packet == NULL)
489-
return 0;
490-
return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
491-
}
492-
493491
static inline size_t
494492
arv_gvcp_packet_get_read_register_ack_size (void)
495493
{
496494
return sizeof (ArvGvcpHeader) + sizeof (guint32);
497495
}
498496

497+
static inline guint32
498+
arv_gvcp_packet_get_read_register_ack_value (const ArvGvcpPacket *packet, size_t packet_size)
499+
{
500+
if G_UNLIKELY(packet == NULL || packet_size < arv_gvcp_packet_get_read_register_ack_size())
501+
return 0;
502+
503+
return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
504+
}
505+
499506
static inline void
500-
arv_gvcp_packet_get_write_register_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *value)
507+
arv_gvcp_packet_get_write_register_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
508+
guint32 *address, guint32 *value)
501509
{
502-
if (packet == NULL) {
510+
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + 2 * sizeof (guint32)) {
503511
if (address != NULL)
504512
*address = 0;
505513
if (value != NULL)
506514
*value = 0;
507515
return;
508516
}
517+
509518
if (address != NULL)
510519
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
511520
if (value != NULL)
@@ -543,9 +552,12 @@ arv_gvcp_packet_get_pending_ack_size (void)
543552
*/
544553

545554
static inline guint32
546-
arv_gvcp_packet_get_pending_ack_timeout (const ArvGvcpPacket *packet)
555+
arv_gvcp_packet_get_pending_ack_timeout (const ArvGvcpPacket *packet, size_t packet_size)
547556
{
548-
return packet != NULL ? g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket)))) : 0;
557+
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32))
558+
return 0;
559+
560+
return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
549561
}
550562

551563
G_END_DECLS

‎src/arvgvdevice.c

+42-36
Original file line numberDiff line numberDiff line change
@@ -243,61 +243,67 @@ _send_cmd_and_receive_ack (ArvGvDeviceIOData *io_data, ArvGvcpCommand command,
243243

244244
arv_gvcp_packet_debug (ack_packet, ARV_DEBUG_LEVEL_TRACE);
245245

246-
packet_type = arv_gvcp_packet_get_packet_type (ack_packet);
247-
ack_command = arv_gvcp_packet_get_command (ack_packet);
248-
packet_id = arv_gvcp_packet_get_packet_id (ack_packet);
246+
packet_type = arv_gvcp_packet_get_packet_type (ack_packet, count);
247+
ack_command = arv_gvcp_packet_get_command (ack_packet, count);
248+
packet_id = arv_gvcp_packet_get_packet_id (ack_packet, count);
249249

250250
if (ack_command == ARV_GVCP_COMMAND_PENDING_ACK &&
251251
count >= arv_gvcp_packet_get_pending_ack_size ()) {
252-
gint64 pending_ack_timeout_ms = arv_gvcp_packet_get_pending_ack_timeout (ack_packet);
252+
gint64 pending_ack_timeout_ms =
253+
arv_gvcp_packet_get_pending_ack_timeout (ack_packet, count);
253254
pending_ack = TRUE;
254255
expected_answer = FALSE;
255256

256257
timeout_stop_ms = g_get_monotonic_time () / 1000 + pending_ack_timeout_ms;
257258

258-
arv_debug_device ("[GvDevice::%s] Pending ack timeout = %" G_GINT64_FORMAT,
259-
operation, pending_ack_timeout_ms);
260-
} else if (packet_type == ARV_GVCP_PACKET_TYPE_ERROR ||
259+
arv_debug_device ("[GvDevice::%s] Pending ack timeout = %"
260+
G_GINT64_FORMAT,
261+
operation, pending_ack_timeout_ms);
262+
} else if (packet_type == ARV_GVCP_PACKET_TYPE_ERROR ||
261263
packet_type == ARV_GVCP_PACKET_TYPE_UNKNOWN_ERROR) {
262-
expected_answer = ack_command == expected_ack_command &&
263-
packet_id == io_data->packet_id;
264-
if (!expected_answer) {
265-
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)", operation,
266-
packet_type);
267-
} else
268-
command_error = arv_gvcp_packet_get_packet_flags (ack_packet);
269-
} else {
270-
expected_answer = packet_type == ARV_GVCP_PACKET_TYPE_ACK &&
271-
ack_command == expected_ack_command &&
272-
packet_id == io_data->packet_id &&
273-
count >= ack_size;
274-
if (!expected_answer) {
275-
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)", operation,
276-
packet_type);
277-
}
278-
}
279-
} else {
280-
expected_answer = FALSE;
281-
if (local_error != NULL)
282-
arv_warning_device ("[GvDevice::%s] Ack reception error: %s", operation,
283-
local_error->message);
284-
else
285-
arv_warning_device ("[GvDevice::%s] Ack reception timeout", operation);
286-
g_clear_error (&local_error);
287-
}
288-
} while (pending_ack || (!expected_answer && timeout_ms > 0));
264+
expected_answer = ack_command == expected_ack_command &&
265+
packet_id == io_data->packet_id;
266+
if (!expected_answer) {
267+
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)",
268+
operation, packet_type);
269+
} else
270+
command_error = arv_gvcp_packet_get_packet_flags (ack_packet,
271+
count);
272+
} else {
273+
expected_answer = packet_type == ARV_GVCP_PACKET_TYPE_ACK &&
274+
ack_command == expected_ack_command &&
275+
packet_id == io_data->packet_id &&
276+
count >= ack_size;
277+
if (!expected_answer) {
278+
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)",
279+
operation, packet_type);
280+
}
281+
}
282+
} else {
283+
expected_answer = FALSE;
284+
if (local_error != NULL)
285+
arv_warning_device ("[GvDevice::%s] Ack reception error: %s", operation,
286+
local_error->message);
287+
else
288+
arv_warning_device ("[GvDevice::%s] Ack reception timeout", operation);
289+
g_clear_error (&local_error);
290+
}
291+
} while (pending_ack || (!expected_answer && timeout_ms > 0));
289292

290293
success = success && expected_answer;
291294

292295
if (success && command_error == ARV_GVCP_ERROR_NONE) {
293296
switch (command) {
294297
case ARV_GVCP_COMMAND_READ_MEMORY_CMD:
295-
memcpy (buffer, arv_gvcp_packet_get_read_memory_ack_data (ack_packet), size);
298+
memcpy (buffer,
299+
arv_gvcp_packet_get_read_memory_ack_data (ack_packet),
300+
size);
296301
break;
297302
case ARV_GVCP_COMMAND_WRITE_MEMORY_CMD:
298303
break;
299304
case ARV_GVCP_COMMAND_READ_REGISTER_CMD:
300-
*((gint32 *) buffer) = arv_gvcp_packet_get_read_register_ack_value (ack_packet);
305+
*((gint32 *) buffer) =
306+
arv_gvcp_packet_get_read_register_ack_value (ack_packet, count);
301307
break;
302308
case ARV_GVCP_COMMAND_WRITE_REGISTER_CMD:
303309
break;

‎src/arvgvfakecamera.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
146146

147147
arv_gvcp_packet_debug (packet, ARV_DEBUG_LEVEL_DEBUG);
148148

149-
packet_id = arv_gvcp_packet_get_packet_id (packet);
150-
packet_type = arv_gvcp_packet_get_packet_type (packet);
149+
packet_id = arv_gvcp_packet_get_packet_id (packet, size);
150+
packet_type = arv_gvcp_packet_get_packet_type (packet, size);
151151

152152
if (packet_type != ARV_GVCP_PACKET_TYPE_CMD) {
153153
arv_warning_device ("[GvFakeCamera::handle_control_packet] Unknown packet type");
@@ -162,7 +162,7 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
162162
&ack_packet->data);
163163
break;
164164
case ARV_GVCP_COMMAND_READ_MEMORY_CMD:
165-
arv_gvcp_packet_get_read_memory_cmd_infos (packet, &block_address, &block_size);
165+
arv_gvcp_packet_get_read_memory_cmd_infos (packet, size, &block_address, &block_size);
166166
arv_info_device ("[GvFakeCamera::handle_control_packet] Read memory command %d (%d)",
167167
block_address, block_size);
168168
ack_packet = arv_gvcp_packet_new_read_memory_ack (block_address, block_size,
@@ -171,9 +171,10 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
171171
arv_gvcp_packet_get_read_memory_ack_data (ack_packet));
172172
break;
173173
case ARV_GVCP_COMMAND_WRITE_MEMORY_CMD:
174-
arv_gvcp_packet_get_write_memory_cmd_infos (packet, &block_address, &block_size);
174+
arv_gvcp_packet_get_write_memory_cmd_infos (packet, size, &block_address, &block_size);
175175
if (!write_access) {
176-
arv_warning_device("[GvFakeCamera::handle_control_packet] Ignore Write memory command %d (%d) not controller",
176+
arv_warning_device("[GvFakeCamera::handle_control_packet]"
177+
" Ignore Write memory command %d (%d) not controller",
177178
block_address, block_size);
178179
break;
179180
}
@@ -186,7 +187,7 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
186187
&ack_packet_size);
187188
break;
188189
case ARV_GVCP_COMMAND_READ_REGISTER_CMD:
189-
arv_gvcp_packet_get_read_register_cmd_infos (packet, &register_address);
190+
arv_gvcp_packet_get_read_register_cmd_infos (packet, size, &register_address);
190191
arv_fake_camera_read_register (gv_fake_camera->priv->camera, register_address, &register_value);
191192
arv_info_device ("[GvFakeCamera::handle_control_packet] Read register command %d -> %d",
192193
register_address, register_value);
@@ -198,9 +199,10 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
198199

199200
break;
200201
case ARV_GVCP_COMMAND_WRITE_REGISTER_CMD:
201-
arv_gvcp_packet_get_write_register_cmd_infos (packet, &register_address, &register_value);
202+
arv_gvcp_packet_get_write_register_cmd_infos (packet, size, &register_address, &register_value);
202203
if (!write_access) {
203-
arv_warning_device("[GvFakeCamera::handle_control_packet] Ignore Write register command %d (%d) not controller",
204+
arv_warning_device("[GvFakeCamera::handle_control_packet]"
205+
" Ignore Write register command %d (%d) not controller",
204206
register_address, register_value);
205207
break;
206208
}

0 commit comments

Comments
 (0)