From 9d99e9c77573560c4f833e7bf4974ac7bb588244 Mon Sep 17 00:00:00 2001
From: Demi Obenour <demi@invisiblethingslab.com>
Date: Sun, 17 Mar 2024 16:05:55 +0000
Subject: [PATCH 1/2] Fetch signature and database from the same URL

Previously, the for loops on lines 1035 and 1037 would advance to the
next element in the server list, even if downloading the URL succeeded.
If there are no more servers in the list, `s` would be NULL, causing
a NULL pointer dereference on line 1046.  If there were servers left
in the list, the signature would be downloaded from a wrong URL.


1. Fetching of database signatures is enabled.
2. There is only one enabled remote repository URL, or fetching from
   all but the last one fails and fetching from the last one succeeds.
3. An XferCommand is used.

Qubes OS Arch templates satisfy all of these conditions and trigger the bug.
---
 lib/libalpm/dload.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 106390a01..8f6b9e4ea 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -1032,13 +1032,18 @@ int _alpm_download(alpm_handle_t *handle,
 					}
 				}
 			} else {
-				for(s = payload->cache_servers; s && ret == -1; s = s->next) {
+				for(s = payload->cache_servers; s; s = s->next) {
 					ret = payload_download_fetchcb(payload, s->data, localpath);
+					if (ret != -1)
+						goto download_signature;
 				}
-				for(s = payload->servers; s && ret == -1; s = s->next) {
+				for(s = payload->servers; s; s = s->next) {
 					ret = payload_download_fetchcb(payload, s->data, localpath);
+					if (ret != -1)
+						goto download_signature;
 				}
 
+download_signature:
 				if (ret != -1 && payload->download_signature) {
 					/* Download signature if requested */
 					char *sig_fileurl;
-- 
GitLab


From 43c9365cfe3bc95f0fb1227fd8a75fe420b2ab52 Mon Sep 17 00:00:00 2001
From: Demi Obenour <demi@invisiblethingslab.com>
Date: Mon, 18 Mar 2024 04:57:26 +0000
Subject: [PATCH 2/2] Use braces around goto statements

No functional change.
---
 lib/libalpm/dload.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/libalpm/dload.c b/lib/libalpm/dload.c
index 8f6b9e4ea..f2fa1a543 100644
--- a/lib/libalpm/dload.c
+++ b/lib/libalpm/dload.c
@@ -1034,13 +1034,15 @@ int _alpm_download(alpm_handle_t *handle,
 			} else {
 				for(s = payload->cache_servers; s; s = s->next) {
 					ret = payload_download_fetchcb(payload, s->data, localpath);
-					if (ret != -1)
+					if (ret != -1) {
 						goto download_signature;
+					}
 				}
 				for(s = payload->servers; s; s = s->next) {
 					ret = payload_download_fetchcb(payload, s->data, localpath);
-					if (ret != -1)
+					if (ret != -1) {
 						goto download_signature;
+					}
 				}
 
 download_signature:
-- 
GitLab

