* [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).