[PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth

[PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

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(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 85da0c0..97961da 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1943,7 +1943,7 @@ static gboolean avrcp_set_browsed_player_rsp(struct avctp *conn,
 
 	items = bt_get_be32(&pdu->params[3]);
 
-	depth = operands[9];
+	depth = pdu->params[9];
 
 	folders = g_new0(char *, depth + 2);
 	folders[0] = g_strdup("/Filesystem");
-- 
1.8.1.4

[PATCH BlueZ 2/5 v2] AVRCP: Fix not parsing player name properly

[Luiz Augusto von Dentz]

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

[PATCH BlueZ 3/5 v2] AVRCP: Fix not checking for invalid folder length

[Luiz Augusto von Dentz]

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

[PATCH BlueZ 4/5 v2] AVRCP: Fix not checking for invalid player name length

[Luiz Augusto von Dentz]

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

[PATCH BlueZ 5/5 v2] AVRCP: Fix not checking for invalid player items

[Luiz Augusto von Dentz]

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

Re: [PATCH BlueZ 1/5 v2] AVRCP: Fix reading wrong field as folder depth

[Johan Hedberg]

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