[PATCH] obexd/bip: Fix uninitialized memory and malformed XML in GetImage request

[PATCH] obexd/bip: Fix uninitialized memory and malformed XML in GetImage request

[maxing]

This commit addresses several defects in the AVRCP BIP GetImage
implementation that caused strict OBEX servers (e.g., Android)
to reject image transfers with "Precondition Failed (0x4C)".

1. Fix uninitialized memory read:
   In `get_image()`, the `maxsize` variable was not initialized. If the
   client omitted the "maxsize" key in the D-Bus dictionary, `maxsize`
   retained garbage data from the stack, leading to anomalous log entries.

2. Fix malformed XML image descriptor:
   When D-Bus clients omitted the `pixel` or `encoding` parameters,
   `parse_get_image_dict()` improperly forced them to empty strings ("").
   This resulted in `get_image()` generating an invalid XML tag:
   `<image encoding="" pixel=""/>`.
   The XML construction logic has been rewritten to dynamically append
   only the provided attributes, and the previously parsed but ignored
   `maxsize` attribute is now properly included if specified.

3. Fix D-Bus iterator advancement:
   Removed a redundant D-Bus type check in `parse_get_image_dict()`
   where the iterator was evaluated twice before calling
   `dbus_message_iter_next()`.

Signed-off-by: maxing <2965346066@qq.com>
---
 obexd/client/bip.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/obexd/client/bip.c b/obexd/client/bip.c
index ec75fdd67..7c724b4b4 100644
--- a/obexd/client/bip.c
+++ b/obexd/client/bip.c
@@ -38,9 +38,6 @@
 
 #define EOL_CHARS "\n"
 #define IMG_DESC_BEGIN "<image-descriptor version=\"1.0\">" EOL_CHARS
-#define IMG_BEGIN "<image encoding=\"%s\" pixel=\"%s\""
-#define IMG_TRANSFORM " transformation=\"%s\""
-#define IMG_END "/>" EOL_CHARS
 #define IMG_DESC_END "</image-descriptor>" EOL_CHARS
 
 static DBusConnection *conn;
@@ -206,9 +203,9 @@ static gboolean parse_get_image_dict(DBusMessage *msg, char **path,
 		goto failed;
 	dbus_message_iter_get_basic(&iter, path);
 	*path = g_strdup(*path);
+	dbus_message_iter_next(&iter);
 	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
 		goto failed;
-	dbus_message_iter_next(&iter);
 	dbus_message_iter_get_basic(&iter, handle);
 	*handle = g_strdup(*handle);
 	dbus_message_iter_next(&iter);
@@ -258,13 +255,11 @@ static gboolean parse_get_image_dict(DBusMessage *msg, char **path,
 		dbus_message_iter_next(&array);
 	}
 
-	if (*pixel == NULL)
-		*pixel = strdup("");
-	if (*encoding == NULL)
-		*encoding = strdup("");
-
-	DBG("pixel: '%s' encoding: '%s' maxsize: '%lu' transform: '%s'",
-			*pixel, *encoding, *maxsize, *transform
+	DBG("pixel: '%s' encoding: '%s' maxsize: '%llu' transform: '%s'",
+			*pixel ? *pixel : "",
+			*encoding ? *encoding : "",
+			(unsigned long long)*maxsize,
+			*transform ? *transform : ""
 	);
 
 	return TRUE;
@@ -283,7 +278,7 @@ static DBusMessage *get_image(DBusConnection *connection,
 	struct bip_avrcp_data *bip_avrcp = user_data;
 	char *handle = NULL, *image_path = NULL, *transform = NULL,
 		*encoding = NULL, *pixel = NULL;
-	uint64_t maxsize;
+	uint64_t maxsize = 0;
 	struct obc_transfer *transfer;
 	GObexHeader *header;
 	DBusMessage *reply = NULL;
@@ -310,10 +305,16 @@ static DBusMessage *get_image(DBusConnection *connection,
 	obc_transfer_add_header(transfer, header);
 
 	descriptor = g_string_new(IMG_DESC_BEGIN);
-	g_string_append_printf(descriptor, IMG_BEGIN, encoding, pixel);
+	g_string_append(descriptor, "<image");
+	if (encoding != NULL)
+		g_string_append_printf(descriptor, " encoding=\"%s\"", encoding);
+	if (pixel != NULL)
+		g_string_append_printf(descriptor, " pixel=\"%s\"", pixel);
+	if (maxsize > 0)
+		g_string_append_printf(descriptor, " maxsize=\"%llu\"", (unsigned long long)maxsize);
 	if (transform != NULL)
-		g_string_append_printf(descriptor, IMG_TRANSFORM, transform);
-	g_string_append(descriptor, IMG_END);
+		g_string_append_printf(descriptor, " transformation=\"%s\"", transform);
+	g_string_append(descriptor, "/>" EOL_CHARS);
 	descriptor = g_string_append(descriptor, IMG_DESC_END);
 	header = g_obex_header_new_bytes(IMG_DESC_TAG, descriptor->str,
 						descriptor->len);
-- 
2.51.0

RE: obexd/bip: Fix uninitialized memory and malformed XML in GetImage request

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1070581

---Test result---

Test Summary:
CheckPatch                    PENDING   0.32 seconds
GitLint                       PENDING   0.34 seconds
BuildEll                      PASS      21.48 seconds
BluezMake                     PASS      647.72 seconds
MakeCheck                     PASS      19.02 seconds
MakeDistcheck                 PASS      255.04 seconds
CheckValgrind                 PASS      302.62 seconds
CheckSmatch                   PASS      364.95 seconds
bluezmakeextell               PASS      186.61 seconds
IncrementalBuild              PENDING   0.32 seconds
ScanBuild                     PASS      1043.65 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



https://github.com/bluez/bluez/pull/1983/checks

---
Regards,
Linux Bluetooth

Re: [PATCH] obexd/bip: Fix uninitialized memory and malformed XML in GetImage request

[Bastien Nocera]

On Mon, 2026-03-23 at 08:48 +0800, maxing wrote:
> This commit addresses several defects in the AVRCP BIP GetImage
> implementation that caused strict OBEX servers (e.g., Android)
> to reject image transfers with "Precondition Failed (0x4C)".
> 
> 1. Fix uninitialized memory read:
>    In `get_image()`, the `maxsize` variable was not initialized. If
> the
>    client omitted the "maxsize" key in the D-Bus dictionary,
> `maxsize`
>    retained garbage data from the stack, leading to anomalous log
> entries.
> 
> 2. Fix malformed XML image descriptor:
>    When D-Bus clients omitted the `pixel` or `encoding` parameters,
>    `parse_get_image_dict()` improperly forced them to empty strings
> ("").
>    This resulted in `get_image()` generating an invalid XML tag:
>    `<image encoding="" pixel=""/>`.
>    The XML construction logic has been rewritten to dynamically
> append
>    only the provided attributes, and the previously parsed but
> ignored
>    `maxsize` attribute is now properly included if specified.
> 
> 3. Fix D-Bus iterator advancement:
>    Removed a redundant D-Bus type check in `parse_get_image_dict()`
>    where the iterator was evaluated twice before calling
>    `dbus_message_iter_next()`.

I think that it would be useful for you to split those 3 fixes into 3
separate commits in a single patchset, which would make it easier to
review individually.

Regards

> 
> Signed-off-by: maxing <2965346066@qq.com>
> ---
>  obexd/client/bip.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/obexd/client/bip.c b/obexd/client/bip.c
> index ec75fdd67..7c724b4b4 100644
> --- a/obexd/client/bip.c
> +++ b/obexd/client/bip.c
> @@ -38,9 +38,6 @@
>  
>  #define EOL_CHARS "\n"
>  #define IMG_DESC_BEGIN "<image-descriptor version=\"1.0\">"
> EOL_CHARS
> -#define IMG_BEGIN "<image encoding=\"%s\" pixel=\"%s\""
> -#define IMG_TRANSFORM " transformation=\"%s\""
> -#define IMG_END "/>" EOL_CHARS
>  #define IMG_DESC_END "</image-descriptor>" EOL_CHARS
>  
>  static DBusConnection *conn;
> @@ -206,9 +203,9 @@ static gboolean parse_get_image_dict(DBusMessage
> *msg, char **path,
>  		goto failed;
>  	dbus_message_iter_get_basic(&iter, path);
>  	*path = g_strdup(*path);
> +	dbus_message_iter_next(&iter);
>  	if (dbus_message_iter_get_arg_type(&iter) !=
> DBUS_TYPE_STRING)
>  		goto failed;
> -	dbus_message_iter_next(&iter);
>  	dbus_message_iter_get_basic(&iter, handle);
>  	*handle = g_strdup(*handle);
>  	dbus_message_iter_next(&iter);
> @@ -258,13 +255,11 @@ static gboolean
> parse_get_image_dict(DBusMessage *msg, char **path,
>  		dbus_message_iter_next(&array);
>  	}
>  
> -	if (*pixel == NULL)
> -		*pixel = strdup("");
> -	if (*encoding == NULL)
> -		*encoding = strdup("");
> -
> -	DBG("pixel: '%s' encoding: '%s' maxsize: '%lu' transform:
> '%s'",
> -			*pixel, *encoding, *maxsize, *transform
> +	DBG("pixel: '%s' encoding: '%s' maxsize: '%llu' transform:
> '%s'",
> +			*pixel ? *pixel : "",
> +			*encoding ? *encoding : "",
> +			(unsigned long long)*maxsize,
> +			*transform ? *transform : ""
>  	);
>  
>  	return TRUE;
> @@ -283,7 +278,7 @@ static DBusMessage *get_image(DBusConnection
> *connection,
>  	struct bip_avrcp_data *bip_avrcp = user_data;
>  	char *handle = NULL, *image_path = NULL, *transform = NULL,
>  		*encoding = NULL, *pixel = NULL;
> -	uint64_t maxsize;
> +	uint64_t maxsize = 0;
>  	struct obc_transfer *transfer;
>  	GObexHeader *header;
>  	DBusMessage *reply = NULL;
> @@ -310,10 +305,16 @@ static DBusMessage *get_image(DBusConnection
> *connection,
>  	obc_transfer_add_header(transfer, header);
>  
>  	descriptor = g_string_new(IMG_DESC_BEGIN);
> -	g_string_append_printf(descriptor, IMG_BEGIN, encoding,
> pixel);
> +	g_string_append(descriptor, "<image");
> +	if (encoding != NULL)
> +		g_string_append_printf(descriptor, "
> encoding=\"%s\"", encoding);
> +	if (pixel != NULL)
> +		g_string_append_printf(descriptor, " pixel=\"%s\"",
> pixel);
> +	if (maxsize > 0)
> +		g_string_append_printf(descriptor, "
> maxsize=\"%llu\"", (unsigned long long)maxsize);
>  	if (transform != NULL)
> -		g_string_append_printf(descriptor, IMG_TRANSFORM,
> transform);
> -	g_string_append(descriptor, IMG_END);
> +		g_string_append_printf(descriptor, "
> transformation=\"%s\"", transform);
> +	g_string_append(descriptor, "/>" EOL_CHARS);
>  	descriptor = g_string_append(descriptor, IMG_DESC_END);
>  	header = g_obex_header_new_bytes(IMG_DESC_TAG, descriptor-
> >str,
>  						descriptor->len);