linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth
@ 2013-03-07 14:57 Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 2/5] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 14:57 UTC (permalink / raw)
  To: linux-bluetooth

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

Folder depth is bit 10 of the parameters not the PDU operands
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH BlueZ 2/5] AVRCP: Fix not parsing player name properly
  2013-03-07 14:57 [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
@ 2013-03-07 14:57 ` Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 3/5] AVRCP: Fix not checking for invalid folder length Luiz Augusto von Dentz
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 14:57 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 | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 97961da..5836233 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,12 @@ 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) {
+		if (namelen > sizeof(name) - 1)
+			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] 5+ messages in thread

* [PATCH BlueZ 3/5] AVRCP: Fix not checking for invalid folder length
  2013-03-07 14:57 [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 2/5] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
@ 2013-03-07 14:57 ` Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 4/5] AVRCP: Fix not checking for invalid player name length Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 5/5] AVRCP: Fix not checking for invalid player items Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 14:57 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 5836233..e059cff 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 || i == 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] 5+ messages in thread

* [PATCH BlueZ 4/5] AVRCP: Fix not checking for invalid player name length
  2013-03-07 14:57 [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 2/5] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 3/5] AVRCP: Fix not checking for invalid folder length Luiz Augusto von Dentz
@ 2013-03-07 14:57 ` Luiz Augusto von Dentz
  2013-03-07 14:57 ` [PATCH BlueZ 5/5] AVRCP: Fix not checking for invalid player items Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 14:57 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 e059cff..e4f83b6 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) {
 		if (namelen > sizeof(name) - 1)
 			namelen = sizeof(name) - 1;
 		memcpy(name, &operands[28], namelen);
-- 
1.8.1.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH BlueZ 5/5] AVRCP: Fix not checking for invalid player items
  2013-03-07 14:57 [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
                   ` (2 preceding siblings ...)
  2013-03-07 14:57 ` [PATCH BlueZ 4/5] AVRCP: Fix not checking for invalid player name length Luiz Augusto von Dentz
@ 2013-03-07 14:57 ` Luiz Augusto von Dentz
  3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2013-03-07 14:57 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 e4f83b6..2a7a401 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -2109,16 +2109,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;
 
@@ -2131,7 +2132,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] 5+ messages in thread

end of thread, other threads:[~2013-03-07 14:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-07 14:57 [PATCH BlueZ 1/5] AVRCP: Fix reading wrong field as folder depth Luiz Augusto von Dentz
2013-03-07 14:57 ` [PATCH BlueZ 2/5] AVRCP: Fix not parsing player name properly Luiz Augusto von Dentz
2013-03-07 14:57 ` [PATCH BlueZ 3/5] AVRCP: Fix not checking for invalid folder length Luiz Augusto von Dentz
2013-03-07 14:57 ` [PATCH BlueZ 4/5] AVRCP: Fix not checking for invalid player name length Luiz Augusto von Dentz
2013-03-07 14:57 ` [PATCH BlueZ 5/5] AVRCP: Fix not checking for invalid player items Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).