On Tue Jul 14, 2026 at 7:58 PM CEST, Junio C Hamano wrote:

Pablo Sabater [off-list ref] writes:

quoted

+	for (size_t i = 0; packet_reader_read(reader) == PACKET_READ_NORMAL && i < args->oids->nr; i++) {

An overly long line.  Format it like this, perhaps?

	for (size_t i = 0;
	     packet_reader_read(reader) == PACKET_READ_NORMAL && i < args->oids->nr;
	     i++) {

or even:

	for (size_t i = 0;
	     packet_reader_read(reader) == PACKET_READ_NORMAL &&
	     i < args->oids->nr;
	     i++) {

Will wrap that line, thanks.

quoted

+		struct string_list object_info_values = STRING_LIST_INIT_DUP;
+
+		string_list_split(&object_info_values, reader->line, " ", -1);
+		if (size_index >= 0) {
+			if (!strcmp(object_info_values.items[1 + size_index].string, "")) {
+				FREE_AND_NULL(object_info_data[i].sizep);
+				string_list_clear(&object_info_values, 0);
+				continue;
+			}
+
+			if (parse_object_size(object_info_values.items[1 + size_index].string,
+					      object_info_data[i].sizep))
+				die("object-info: ref %s has invalid size %s",
+				    object_info_values.items[0].string,
+				    object_info_values.items[1 + size_index].string);
+		}
+
+		string_list_clear(&object_info_values, 0);

Is this not trusting the other side too much?

If the other end returns fewer values than expected (e.g., if a
buggy or malicious server returns only "<oid>" without a trailing
space for an unrecognized object, or if we request multiple
attributes in the future and the server returns fewer values than
expected), string_list_split may return a list with fewer elements
than size_index + 1.  Accessing object_info_values.items[size_index
+ 1] will then result in an out-of-bounds read/crash.

I will add a check for a malformed response from the server so "<oid>" is
considered corrupted, similar to the size values a few lines below.

A subsequent commit in this series (13) adds a filter that drops
attributes requested by the client but that the server doesn't support so
we should expect full return of the attributes asked or "<oid> SP".

I'll add a guard just in case in a future what we expect changes.

By the way, from a stylistic standpoint, "size_index + 1" reads a
bit more naturally than the "1 + size_index" used in the current
patch.

Will change it.

Thanks for the feedback,
Pablo.