[pacman-dev] [PATCH] ensure matching database and package version

2015-07-18 Thread Levente Polyak
While loading each package ensure that the internal version matches the
expected database version to avoid the possibility to circumvent the
version check.
This issue can be used by an attacker to trick the software into
installing an older version. The behavior can be  exploited by a
man-in-the-middle attack through specially crafted  database tarball
containing a higher version, yet actually delivering an  older and
vulnerable version, which was previously shipped.

Signed-off-by: Levente Polyak 
Signed-off-by: Remi Gacogne 
---
 lib/libalpm/sync.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
index 888ae15..8504e02 100644
--- a/lib/libalpm/sync.c
+++ b/lib/libalpm/sync.c
@@ -1212,6 +1212,7 @@ static int load_packages(alpm_handle_t *handle, 
alpm_list_t **data,
EVENT(handle, &event);

for(i = handle->trans->add; i; i = i->next, current++) {
+   int error = 0;
alpm_pkg_t *spkg = i->data;
char *filepath;
int percent = (int)(((double)current_bytes / total_bytes) * 
100);
@@ -1232,6 +1233,21 @@ static int load_packages(alpm_handle_t *handle, 
alpm_list_t **data,
spkg->name);
alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 
1);
if(!pkgfile) {
+   _alpm_log(handle, ALPM_LOG_DEBUG, "failed to load 
pkgfile internal\n");
+   error = 1;
+   } else {
+   if(strcmp(spkg->name, pkgfile->name) != 0) {
+   _alpm_log(handle, ALPM_LOG_DEBUG, "internal 
package name missmatch, expected: '%s', actual: '%s'\n",
+   spkg->name, pkgfile->name);
+   error = 1;
+   }
+   if(alpm_pkg_vercmp(spkg->version, pkgfile->version) != 
0) {
+   _alpm_log(handle, ALPM_LOG_DEBUG, "internal 
package version missmatch, expected: '%s', actual: '%s'\n",
+   spkg->version, 
pkgfile->version);
+   error = 1;
+   }
+   }
+   if(error != 0) {
errors++;
*data = alpm_list_add(*data, strdup(spkg->filename));
free(filepath);
-- 
2.4.6


Re: [pacman-dev] [PATCH] ensure matching database and package version

2015-07-18 Thread Jens Adam
Nice. :]
Context: https://bugs.archlinux.org/task/45657

--byte



pgpCpzERAbgvX.pgp
Description: Digitale Signatur von OpenPGP


Re: [pacman-dev] [PATCH] ensure matching database and package version

2015-07-18 Thread Doug Newgard
On Sat, 18 Jul 2015 17:03:01 +0200
Jens Adam  wrote:

> Nice. :]
> Context: https://bugs.archlinux.org/task/45657
> 
> --byte
> 

This is more towards https://bugs.archlinux.org/task/45687


Re: [pacman-dev] [PATCH] ensure matching database and package version

2015-07-18 Thread Andrew Gregory
On 07/18/15 at 04:55pm, Levente Polyak wrote:
> While loading each package ensure that the internal version matches the
> expected database version to avoid the possibility to circumvent the
> version check.
> This issue can be used by an attacker to trick the software into
> installing an older version. The behavior can be  exploited by a
> man-in-the-middle attack through specially crafted  database tarball
> containing a higher version, yet actually delivering an  older and
> vulnerable version, which was previously shipped.
> 
> Signed-off-by: Levente Polyak 
> Signed-off-by: Remi Gacogne 
> ---
>  lib/libalpm/sync.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index 888ae15..8504e02 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -1212,6 +1212,7 @@ static int load_packages(alpm_handle_t *handle, 
> alpm_list_t **data,
>   EVENT(handle, &event);
> 
>   for(i = handle->trans->add; i; i = i->next, current++) {
> + int error = 0;
>   alpm_pkg_t *spkg = i->data;
>   char *filepath;
>   int percent = (int)(((double)current_bytes / total_bytes) * 
> 100);
> @@ -1232,6 +1233,21 @@ static int load_packages(alpm_handle_t *handle, 
> alpm_list_t **data,
>   spkg->name);
>   alpm_pkg_t *pkgfile =_alpm_pkg_load_internal(handle, filepath, 
> 1);
>   if(!pkgfile) {
> + _alpm_log(handle, ALPM_LOG_DEBUG, "failed to load 
> pkgfile internal\n");
> + error = 1;
> + } else {
> + if(strcmp(spkg->name, pkgfile->name) != 0) {
> + _alpm_log(handle, ALPM_LOG_DEBUG, "internal 
> package name missmatch, expected: '%s', actual: '%s'\n",

s/missmatch/mismatch/.  Also, we try to stay close to 80 columns,
please wrap these long lines.

> + spkg->name, pkgfile->name);
> + error = 1;
> + }
> + if(alpm_pkg_vercmp(spkg->version, pkgfile->version) != 
> 0) {

I don't think there's any reason to support different version strings
even if they are equivalent, an ordinary strcmp should suffice.

> + _alpm_log(handle, ALPM_LOG_DEBUG, "internal 
> package version mismatch, expected: '%s', actual: '%s'\n",
> + spkg->version, 
> pkgfile->version);

Same as above.

> + error = 1;
> + }
> + }
> + if(error != 0) {
>   errors++;
>   *data = alpm_list_add(*data, strdup(spkg->filename));
>   free(filepath);
> -- 
> 2.4.6