Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gv packet parsing safety checks #977

Merged
merged 2 commits into from
Jan 23, 2025
Merged

Gv packet parsing safety checks #977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8e64776
gvcp: avoid out of bound memory access by checking packet size
EmmanuelP Nov 22, 2024
c7db5b3
ci: update artifact version to v4
EmmanuelP Jan 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/aravis-linux.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
run: meson test -C build/ -v
- name: Valgrind
run: meson test -C build/ -v --setup=valgrind
- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: failure()
with:
name: Linux_Meson_Testlog
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/aravis-macos.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
CC: gcc
- name: Tests
run: meson test -C build/ -v
- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: failure()
with:
name: MacOS_Meson_Testlog
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/aravis-mingw.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ jobs:
- name: meson test
run: |
meson test -C ./build
- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: failure()
with:
name: ${{matrix.sys}}_Meson_Testlog
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/aravis-msvc.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ jobs:
.\build\activate_run.ps1
meson test -C .\build
- name: logs
uses: actions/upload-artifact@v3
uses: actions/upload-artifact@v4
if: failure()
with:
name: msvc_${{ matrix.version }}_${{ matrix.arch }}_${{ matrix.build_type_meson }}_Meson_Testlog
Expand Down
80 changes: 46 additions & 34 deletions src/arvgvcpprivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,9 +357,9 @@ void arv_gvcp_packet_debug (const ArvGvcpPacket *packet, ArvDebugLevel lev
*/

static inline ArvGvcpPacketType
arv_gvcp_packet_get_packet_type (ArvGvcpPacket *packet)
arv_gvcp_packet_get_packet_type (ArvGvcpPacket *packet, size_t size)
{
if (packet == NULL)
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
return ARV_GVCP_PACKET_TYPE_ERROR;

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

static inline guint8
arv_gvcp_packet_get_packet_flags (ArvGvcpPacket *packet)
arv_gvcp_packet_get_packet_flags (ArvGvcpPacket *packet, size_t size)
{
if (packet == NULL)
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
return 0;

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

static inline ArvGvcpCommand
arv_gvcp_packet_get_command (ArvGvcpPacket *packet)
arv_gvcp_packet_get_command (ArvGvcpPacket *packet, size_t size)
{
if (packet == NULL)
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
return (ArvGvcpCommand) 0;

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

static inline void
arv_gvcp_packet_set_packet_id (ArvGvcpPacket *packet, guint16 id)
arv_gvcp_packet_set_packet_id (ArvGvcpPacket *packet, guint16 id, size_t size)
{
if (packet != NULL)
if G_UNLIKELY(packet != NULL && size >= sizeof (ArvGvcpPacket))
packet->header.id = g_htons (id);
}

static inline guint16
arv_gvcp_packet_get_packet_id (ArvGvcpPacket *packet)
arv_gvcp_packet_get_packet_id (ArvGvcpPacket *packet, size_t size)
{
if (packet == NULL)
if G_UNLIKELY(packet == NULL || size < sizeof (ArvGvcpPacket))
return 0;

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

static inline void
arv_gvcp_packet_get_read_memory_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *size)
arv_gvcp_packet_get_read_memory_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
guint32 *address, guint32 *size)
{
if (packet == NULL) {
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + 2 * sizeof (guint32)) {
if (address != NULL)
*address = 0;
if (size != NULL)
*size = 0;
return;
}

if (address != NULL)
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
if (size != NULL)
Expand All @@ -443,69 +445,76 @@ arv_gvcp_packet_get_read_memory_ack_data (const ArvGvcpPacket *packet)
}

static inline void
arv_gvcp_packet_get_write_memory_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *size)
arv_gvcp_packet_get_write_memory_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
guint32 *address, guint32 *size)
{
if (packet == NULL) {
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32)) {
if (address != NULL)
*address = 0;
if (size != NULL)
*size = 0;
return;
}

if (address != NULL)
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
if (size != NULL)
*size = g_ntohs (packet->header.size) - sizeof (guint32);
}

static inline void *
arv_gvcp_packet_get_write_memory_cmd_data (const ArvGvcpPacket *packet)
{
return (char *) packet + sizeof (ArvGvcpPacket) + sizeof (guint32);
}

static inline size_t
arv_gvcp_packet_get_write_memory_ack_size (void)
{
return sizeof (ArvGvcpPacket) + sizeof (guint32);
}

static inline void *
arv_gvcp_packet_get_write_memory_cmd_data (const ArvGvcpPacket *packet)
{
return (char *) packet + sizeof (ArvGvcpPacket) + sizeof (guint32);
}

static inline void
arv_gvcp_packet_get_read_register_cmd_infos (const ArvGvcpPacket *packet, guint32 *address)
arv_gvcp_packet_get_read_register_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
guint32 *address)
{
if (packet == NULL) {
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32)) {
if (address != NULL)
*address = 0;
return;
}

if (address != NULL)
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
}

static inline guint32
arv_gvcp_packet_get_read_register_ack_value (const ArvGvcpPacket *packet)
{
if (packet == NULL)
return 0;
return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
}

static inline size_t
arv_gvcp_packet_get_read_register_ack_size (void)
{
return sizeof (ArvGvcpHeader) + sizeof (guint32);
}

static inline guint32
arv_gvcp_packet_get_read_register_ack_value (const ArvGvcpPacket *packet, size_t packet_size)
{
if G_UNLIKELY(packet == NULL || packet_size < arv_gvcp_packet_get_read_register_ack_size())
return 0;

return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
}

static inline void
arv_gvcp_packet_get_write_register_cmd_infos (const ArvGvcpPacket *packet, guint32 *address, guint32 *value)
arv_gvcp_packet_get_write_register_cmd_infos (const ArvGvcpPacket *packet, size_t packet_size,
guint32 *address, guint32 *value)
{
if (packet == NULL) {
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + 2 * sizeof (guint32)) {
if (address != NULL)
*address = 0;
if (value != NULL)
*value = 0;
return;
}

if (address != NULL)
*address = g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
if (value != NULL)
Expand Down Expand Up @@ -543,9 +552,12 @@ arv_gvcp_packet_get_pending_ack_size (void)
*/

static inline guint32
arv_gvcp_packet_get_pending_ack_timeout (const ArvGvcpPacket *packet)
arv_gvcp_packet_get_pending_ack_timeout (const ArvGvcpPacket *packet, size_t packet_size)
{
return packet != NULL ? g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket)))) : 0;
if G_UNLIKELY(packet == NULL || packet_size < sizeof (ArvGvcpPacket) + sizeof (guint32))
return 0;

return g_ntohl (*((guint32 *) ((char *) packet + sizeof (ArvGvcpPacket))));
}

G_END_DECLS
Expand Down
78 changes: 42 additions & 36 deletions src/arvgvdevice.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,61 +243,67 @@ _send_cmd_and_receive_ack (ArvGvDeviceIOData *io_data, ArvGvcpCommand command,

arv_gvcp_packet_debug (ack_packet, ARV_DEBUG_LEVEL_TRACE);

packet_type = arv_gvcp_packet_get_packet_type (ack_packet);
ack_command = arv_gvcp_packet_get_command (ack_packet);
packet_id = arv_gvcp_packet_get_packet_id (ack_packet);
packet_type = arv_gvcp_packet_get_packet_type (ack_packet, count);
ack_command = arv_gvcp_packet_get_command (ack_packet, count);
packet_id = arv_gvcp_packet_get_packet_id (ack_packet, count);

if (ack_command == ARV_GVCP_COMMAND_PENDING_ACK &&
count >= arv_gvcp_packet_get_pending_ack_size ()) {
gint64 pending_ack_timeout_ms = arv_gvcp_packet_get_pending_ack_timeout (ack_packet);
gint64 pending_ack_timeout_ms =
arv_gvcp_packet_get_pending_ack_timeout (ack_packet, count);
pending_ack = TRUE;
expected_answer = FALSE;

timeout_stop_ms = g_get_monotonic_time () / 1000 + pending_ack_timeout_ms;

arv_debug_device ("[GvDevice::%s] Pending ack timeout = %" G_GINT64_FORMAT,
operation, pending_ack_timeout_ms);
} else if (packet_type == ARV_GVCP_PACKET_TYPE_ERROR ||
arv_debug_device ("[GvDevice::%s] Pending ack timeout = %"
G_GINT64_FORMAT,
operation, pending_ack_timeout_ms);
} else if (packet_type == ARV_GVCP_PACKET_TYPE_ERROR ||
packet_type == ARV_GVCP_PACKET_TYPE_UNKNOWN_ERROR) {
expected_answer = ack_command == expected_ack_command &&
packet_id == io_data->packet_id;
if (!expected_answer) {
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)", operation,
packet_type);
} else
command_error = arv_gvcp_packet_get_packet_flags (ack_packet);
} else {
expected_answer = packet_type == ARV_GVCP_PACKET_TYPE_ACK &&
ack_command == expected_ack_command &&
packet_id == io_data->packet_id &&
count >= ack_size;
if (!expected_answer) {
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)", operation,
packet_type);
}
}
} else {
expected_answer = FALSE;
if (local_error != NULL)
arv_warning_device ("[GvDevice::%s] Ack reception error: %s", operation,
local_error->message);
else
arv_warning_device ("[GvDevice::%s] Ack reception timeout", operation);
g_clear_error (&local_error);
}
} while (pending_ack || (!expected_answer && timeout_ms > 0));
expected_answer = ack_command == expected_ack_command &&
packet_id == io_data->packet_id;
if (!expected_answer) {
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)",
operation, packet_type);
} else
command_error = arv_gvcp_packet_get_packet_flags (ack_packet,
count);
} else {
expected_answer = packet_type == ARV_GVCP_PACKET_TYPE_ACK &&
ack_command == expected_ack_command &&
packet_id == io_data->packet_id &&
count >= ack_size;
if (!expected_answer) {
arv_info_device ("[GvDevice::%s] Unexpected answer (0x%02x)",
operation, packet_type);
}
}
} else {
expected_answer = FALSE;
if (local_error != NULL)
arv_warning_device ("[GvDevice::%s] Ack reception error: %s", operation,
local_error->message);
else
arv_warning_device ("[GvDevice::%s] Ack reception timeout", operation);
g_clear_error (&local_error);
}
} while (pending_ack || (!expected_answer && timeout_ms > 0));

success = success && expected_answer;

if (success && command_error == ARV_GVCP_ERROR_NONE) {
switch (command) {
case ARV_GVCP_COMMAND_READ_MEMORY_CMD:
memcpy (buffer, arv_gvcp_packet_get_read_memory_ack_data (ack_packet), size);
memcpy (buffer,
arv_gvcp_packet_get_read_memory_ack_data (ack_packet),
size);
break;
case ARV_GVCP_COMMAND_WRITE_MEMORY_CMD:
break;
case ARV_GVCP_COMMAND_READ_REGISTER_CMD:
*((gint32 *) buffer) = arv_gvcp_packet_get_read_register_ack_value (ack_packet);
*((gint32 *) buffer) =
arv_gvcp_packet_get_read_register_ack_value (ack_packet, count);
break;
case ARV_GVCP_COMMAND_WRITE_REGISTER_CMD:
break;
Expand Down
18 changes: 10 additions & 8 deletions src/arvgvfakecamera.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,8 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,

arv_gvcp_packet_debug (packet, ARV_DEBUG_LEVEL_DEBUG);

packet_id = arv_gvcp_packet_get_packet_id (packet);
packet_type = arv_gvcp_packet_get_packet_type (packet);
packet_id = arv_gvcp_packet_get_packet_id (packet, size);
packet_type = arv_gvcp_packet_get_packet_type (packet, size);

if (packet_type != ARV_GVCP_PACKET_TYPE_CMD) {
arv_warning_device ("[GvFakeCamera::handle_control_packet] Unknown packet type");
Expand All @@ -162,7 +162,7 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
&ack_packet->data);
break;
case ARV_GVCP_COMMAND_READ_MEMORY_CMD:
arv_gvcp_packet_get_read_memory_cmd_infos (packet, &block_address, &block_size);
arv_gvcp_packet_get_read_memory_cmd_infos (packet, size, &block_address, &block_size);
arv_info_device ("[GvFakeCamera::handle_control_packet] Read memory command %d (%d)",
block_address, block_size);
ack_packet = arv_gvcp_packet_new_read_memory_ack (block_address, block_size,
Expand All @@ -171,9 +171,10 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
arv_gvcp_packet_get_read_memory_ack_data (ack_packet));
break;
case ARV_GVCP_COMMAND_WRITE_MEMORY_CMD:
arv_gvcp_packet_get_write_memory_cmd_infos (packet, &block_address, &block_size);
arv_gvcp_packet_get_write_memory_cmd_infos (packet, size, &block_address, &block_size);
if (!write_access) {
arv_warning_device("[GvFakeCamera::handle_control_packet] Ignore Write memory command %d (%d) not controller",
arv_warning_device("[GvFakeCamera::handle_control_packet]"
" Ignore Write memory command %d (%d) not controller",
block_address, block_size);
break;
}
Expand All @@ -186,7 +187,7 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,
&ack_packet_size);
break;
case ARV_GVCP_COMMAND_READ_REGISTER_CMD:
arv_gvcp_packet_get_read_register_cmd_infos (packet, &register_address);
arv_gvcp_packet_get_read_register_cmd_infos (packet, size, &register_address);
arv_fake_camera_read_register (gv_fake_camera->priv->camera, register_address, &register_value);
arv_info_device ("[GvFakeCamera::handle_control_packet] Read register command %d -> %d",
register_address, register_value);
Expand All @@ -198,9 +199,10 @@ _handle_control_packet (ArvGvFakeCamera *gv_fake_camera, GSocket *socket,

break;
case ARV_GVCP_COMMAND_WRITE_REGISTER_CMD:
arv_gvcp_packet_get_write_register_cmd_infos (packet, &register_address, &register_value);
arv_gvcp_packet_get_write_register_cmd_infos (packet, size, &register_address, &register_value);
if (!write_access) {
arv_warning_device("[GvFakeCamera::handle_control_packet] Ignore Write register command %d (%d) not controller",
arv_warning_device("[GvFakeCamera::handle_control_packet]"
" Ignore Write register command %d (%d) not controller",
register_address, register_value);
break;
}
Expand Down
Loading
Loading