* [PATCH BlueZ 2/5 v2] AVRCP: Fix not parsing player name properly
2013-03-07 15:11 [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
@ 2013-03-07 15:11 ` Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 3/5 v2] AVRCP: Fix not checking for invalid folder length Luiz Augusto von Dentz
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 15:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Player name length field is 2 bytes long not 1.
---
profiles/audio/avrcp.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 97961da..d5e6574 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2058,7 +2058,7 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
{
struct avrcp_player *player = session->player;
struct media_player *mp = player->user_data;
- uint16_t id;
+ uint16_t id, namelen;
uint32_t subtype;
const char *curval, *strval;
char name[255];
@@ -2087,8 +2087,11 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
avrcp_player_parse_features(player, &operands[8]);
- if (operands[26] != 0) {
- memcpy(name, &operands[27], operands[26]);
+ namelen = bt_get_be16(&operands[26]);
+ if (namelen != 0) {
+ namelen = MIN(namelen, sizeof(name) - 1);
+ memcpy(name, &operands[28], namelen);
+ name[namelen] = '\0';
media_player_set_name(mp, name);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH BlueZ 3/5 v2] AVRCP: Fix not checking for invalid folder length
2013-03-07 15:11 [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 2/5 v2] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
@ 2013-03-07 15:11 ` Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 4/5 v2] AVRCP: Fix not checking for invalid player name length Luiz Augusto von Dentz
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 15:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds checks for invalid folder length in SetBrowsedPlayer that
could cause crashes while reading invalid memory.
---
profiles/audio/avrcp.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index d5e6574..afb8c09 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1934,7 +1934,7 @@ static gboolean avrcp_set_browsed_player_rsp(struct avctp *conn,
uint32_t items;
char **folders, *path;
uint8_t depth, count;
- int i;
+ size_t i;
if (pdu->params[0] != AVRCP_STATUS_SUCCESS || operand_count < 13)
return FALSE;
@@ -1948,14 +1948,19 @@ static gboolean avrcp_set_browsed_player_rsp(struct avctp *conn,
folders = g_new0(char *, depth + 2);
folders[0] = g_strdup("/Filesystem");
- for (i = 10, count = 1; count - 1 < depth; count++) {
- char *part;
+ for (i = 10, count = 1; count - 1 < depth && i < operand_count;
+ count++) {
uint8_t len;
len = pdu->params[i++];
- part = g_memdup(&pdu->params[i], len);
+
+ if (i + len > operand_count || len == 0) {
+ error("Invalid folder length");
+ break;
+ }
+
+ folders[count] = g_memdup(&pdu->params[i], len);
i += len;
- folders[count] = part;
}
path = g_build_pathv("/", folders);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH BlueZ 4/5 v2] AVRCP: Fix not checking for invalid player name length
2013-03-07 15:11 [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 2/5 v2] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 3/5 v2] AVRCP: Fix not checking for invalid folder length Luiz Augusto von Dentz
@ 2013-03-07 15:11 ` Luiz Augusto von Dentz
2013-03-07 15:11 ` [PATCH BlueZ 5/5 v2] AVRCP: Fix not checking for invalid player items Luiz Augusto von Dentz
2013-03-07 15:21 ` [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Johan Hedberg
4 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 15:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds checks for invalid player name length that could cause crashes
while reading invalid memory.
---
profiles/audio/avrcp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index afb8c09..192a466 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2093,7 +2093,7 @@ static void avrcp_parse_media_player_item(struct avrcp *session,
avrcp_player_parse_features(player, &operands[8]);
namelen = bt_get_be16(&operands[26]);
- if (namelen != 0) {
+ if (namelen != 0 && namelen + 28 == len) {
namelen = MIN(namelen, sizeof(name) - 1);
memcpy(name, &operands[28], namelen);
name[namelen] = '\0';
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH BlueZ 5/5 v2] AVRCP: Fix not checking for invalid player items
2013-03-07 15:11 [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
` (2 preceding siblings ...)
2013-03-07 15:11 ` [PATCH BlueZ 4/5 v2] AVRCP: Fix not checking for invalid player name length Luiz Augusto von Dentz
@ 2013-03-07 15:11 ` Luiz Augusto von Dentz
2013-03-07 15:21 ` [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Johan Hedberg
4 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 15:11 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
This adds checks for invalid size in the media player list returned by
GetFolderItems that could cause crashes.
---
profiles/audio/avrcp.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 192a466..e3dbb6b 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2108,16 +2108,17 @@ static gboolean avrcp_get_media_player_list_rsp(struct avctp *conn,
size_t operand_count,
void *user_data)
{
+ struct avrcp_browsing_header *pdu = (void *) operands;
struct avrcp *session = user_data;
uint16_t count;
- int i;
+ size_t i;
- if (operands[3] != AVRCP_STATUS_SUCCESS || operand_count < 5)
+ if (pdu->params[0] != AVRCP_STATUS_SUCCESS || operand_count < 5)
return FALSE;
count = bt_get_be16(&operands[6]);
- for (i = 8; count; count--) {
+ for (i = 8; count && i < operand_count; count--) {
uint8_t type;
uint16_t len;
@@ -2130,7 +2131,14 @@ static gboolean avrcp_get_media_player_list_rsp(struct avctp *conn,
continue;
}
+ if (i + len > operand_count) {
+ error("Invalid player item length");
+ return FALSE;
+ }
+
avrcp_parse_media_player_item(session, &operands[i], len);
+
+ i += len;
}
return FALSE;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth
2013-03-07 15:11 [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
` (3 preceding siblings ...)
2013-03-07 15:11 ` [PATCH BlueZ 5/5 v2] AVRCP: Fix not checking for invalid player items Luiz Augusto von Dentz
@ 2013-03-07 15:21 ` Johan Hedberg
4 siblings, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2013-03-07 15:21 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On Thu, Mar 07, 2013, Luiz Augusto von Dentz wrote:
> Folder depth is bit 10 of the parameters not the PDU operands
> ---
> v2: Make use of MIN macro to check player name length and fix zero length
> check for folder.
>
> profiles/audio/avrcp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
This patch set has been applied. Thanks.
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread