gnu: glib: Fix CVE-2021-28153.
* gnu/packages/patches/glib-CVE-2021-28153.patch: New file. * gnu/local.mk (dist_patch_DATA): Add it. * gnu/packages/glib.scm (glib/fixed): Add the new patch.
This commit is contained in:
		
							parent
							
								
									c3a7537396
								
							
						
					
					
						commit
						5a06b83fc9
					
				
					 3 changed files with 286 additions and 1 deletions
				
			
		|  | @ -1095,6 +1095,7 @@ dist_patch_DATA =						\ | |||
|   %D%/packages/patches/glib-CVE-2021-27219-16.patch		\
 | ||||
|   %D%/packages/patches/glib-CVE-2021-27219-17.patch		\
 | ||||
|   %D%/packages/patches/glib-CVE-2021-27219-18.patch		\
 | ||||
|   %D%/packages/patches/glib-CVE-2021-28153.patch		\
 | ||||
|   %D%/packages/patches/glibc-CVE-2018-11236.patch		\
 | ||||
|   %D%/packages/patches/glibc-CVE-2018-11237.patch		\
 | ||||
|   %D%/packages/patches/glibc-CVE-2019-7309.patch		\
 | ||||
|  |  | |||
|  | @ -415,7 +415,8 @@ dynamic loading, and an object system.") | |||
|                                        "glib-CVE-2021-27219-15.patch" | ||||
|                                        "glib-CVE-2021-27219-16.patch" | ||||
|                                        "glib-CVE-2021-27219-17.patch" | ||||
|                                        "glib-CVE-2021-27219-18.patch") | ||||
|                                        "glib-CVE-2021-27219-18.patch" | ||||
|                                        "glib-CVE-2021-28153.patch") | ||||
|                        (origin-patches (package-source glib)))))))) | ||||
| 
 | ||||
| (define-public glib-with-documentation | ||||
|  |  | |||
							
								
								
									
										283
									
								
								gnu/packages/patches/glib-CVE-2021-28153.patch
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										283
									
								
								gnu/packages/patches/glib-CVE-2021-28153.patch
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,283 @@ | |||
| Backport of: | ||||
| 
 | ||||
| From 317b3b587058a05dca95d56dac26568c5b098d33 Mon Sep 17 00:00:00 2001 | ||||
| From: Philip Withnall <pwithnall@endlessos.org> | ||||
| Date: Wed, 24 Feb 2021 17:35:40 +0000 | ||||
| Subject: [PATCH] glocalfileoutputstream: Fix CREATE_REPLACE_DESTINATION | ||||
|  with symlinks | ||||
| MIME-Version: 1.0 | ||||
| Content-Type: text/plain; charset=UTF-8 | ||||
| Content-Transfer-Encoding: 8bit | ||||
| 
 | ||||
| The `G_FILE_CREATE_REPLACE_DESTINATION` flag is equivalent to unlinking | ||||
| the destination file and re-creating it from scratch. That did | ||||
| previously work, but in the process the code would call `open(O_CREAT)` | ||||
| on the file. If the file was a dangling symlink, this would create the | ||||
| destination file (empty). That’s not an intended side-effect, and has | ||||
| security implications if the symlink is controlled by a lower-privileged | ||||
| process. | ||||
| 
 | ||||
| Fix that by not opening the destination file if it’s a symlink, and | ||||
| adjusting the rest of the code to cope with | ||||
|  - the fact that `fd == -1` is not an error iff `is_symlink` is true, | ||||
|  - and that `original_stat` will contain the `lstat()` results for the | ||||
|    symlink now, rather than the `stat()` results for its target (again, | ||||
|    iff `is_symlink` is true). | ||||
| 
 | ||||
| This means that the target of the dangling symlink is no longer created, | ||||
| which was the bug. The symlink itself continues to be replaced (as | ||||
| before) with the new file — this is the intended behaviour of | ||||
| `g_file_replace()`. | ||||
| 
 | ||||
| The behaviour for non-symlink cases, or cases where the symlink was not | ||||
| dangling, should be unchanged. | ||||
| 
 | ||||
| Includes a unit test. | ||||
| 
 | ||||
| Signed-off-by: Philip Withnall <pwithnall@endlessos.org> | ||||
| 
 | ||||
| Fixes: #2325 | ||||
| ---
 | ||||
|  gio/glocalfileoutputstream.c |  70 ++++++++++++++++------- | ||||
|  gio/tests/file.c             | 108 +++++++++++++++++++++++++++++++++++ | ||||
|  2 files changed, 158 insertions(+), 20 deletions(-) | ||||
| 
 | ||||
| diff --git a/gio/glocalfileoutputstream.c b/gio/glocalfileoutputstream.c
 | ||||
| index a3dd62172..553fcbbae 100644
 | ||||
| --- a/gio/glocalfileoutputstream.c
 | ||||
| +++ b/gio/glocalfileoutputstream.c
 | ||||
| @@ -874,16 +874,22 @@ handle_overwrite_open (const char    *filename,
 | ||||
|        /* Could be a symlink, or it could be a regular ELOOP error, | ||||
|         * but then the next open will fail too. */ | ||||
|        is_symlink = TRUE; | ||||
| -      fd = g_open (filename, open_flags, mode);
 | ||||
| +      if (!(flags & G_FILE_CREATE_REPLACE_DESTINATION))
 | ||||
| +        fd = g_open (filename, open_flags, mode);
 | ||||
|      } | ||||
| -#else
 | ||||
| -  fd = g_open (filename, open_flags, mode);
 | ||||
| -  errsv = errno;
 | ||||
| +#else  /* if !O_NOFOLLOW */
 | ||||
|    /* This is racy, but we do it as soon as possible to minimize the race */ | ||||
|    is_symlink = g_file_test (filename, G_FILE_TEST_IS_SYMLINK); | ||||
| +
 | ||||
| +  if (!is_symlink || !(flags & G_FILE_CREATE_REPLACE_DESTINATION))
 | ||||
| +    {
 | ||||
| +      fd = g_open (filename, open_flags, mode);
 | ||||
| +      errsv = errno;
 | ||||
| +    }
 | ||||
|  #endif | ||||
|   | ||||
| -  if (fd == -1)
 | ||||
| +  if (fd == -1 &&
 | ||||
| +      (!is_symlink || !(flags & G_FILE_CREATE_REPLACE_DESTINATION)))
 | ||||
|      { | ||||
|        char *display_name = g_filename_display_name (filename); | ||||
|        g_set_error (error, G_IO_ERROR, | ||||
| @@ -893,13 +899,25 @@ handle_overwrite_open (const char    *filename,
 | ||||
|        g_free (display_name); | ||||
|        return -1; | ||||
|      } | ||||
| -  
 | ||||
| +
 | ||||
| +  if (!is_symlink)
 | ||||
| +    {
 | ||||
|  #ifdef G_OS_WIN32 | ||||
| -  res = GLIB_PRIVATE_CALL (g_win32_fstat) (fd, &original_stat);
 | ||||
| +      res = GLIB_PRIVATE_CALL (g_win32_fstat) (fd, &original_stat);
 | ||||
|  #else | ||||
| -  res = fstat (fd, &original_stat);
 | ||||
| +      res = fstat (fd, &original_stat);
 | ||||
|  #endif | ||||
| -  errsv = errno;
 | ||||
| +      errsv = errno;
 | ||||
| +    }
 | ||||
| +  else
 | ||||
| +    {
 | ||||
| +#ifdef G_OS_WIN32
 | ||||
| +      res = GLIB_PRIVATE_CALL (g_win32_lstat_utf8) (filename, &original_stat);
 | ||||
| +#else
 | ||||
| +      res = g_lstat (filename, &original_stat);
 | ||||
| +#endif
 | ||||
| +      errsv = errno;
 | ||||
| +    }
 | ||||
|   | ||||
|    if (res != 0) | ||||
|      { | ||||
| @@ -916,16 +934,27 @@ handle_overwrite_open (const char    *filename,
 | ||||
|    if (!S_ISREG (original_stat.st_mode)) | ||||
|      { | ||||
|        if (S_ISDIR (original_stat.st_mode)) | ||||
| -	g_set_error_literal (error,
 | ||||
| -                             G_IO_ERROR,
 | ||||
| -                             G_IO_ERROR_IS_DIRECTORY,
 | ||||
| -                             _("Target file is a directory"));
 | ||||
| -      else
 | ||||
| -	g_set_error_literal (error,
 | ||||
| -                             G_IO_ERROR,
 | ||||
| -                             G_IO_ERROR_NOT_REGULAR_FILE,
 | ||||
| -                             _("Target file is not a regular file"));
 | ||||
| -      goto err_out;
 | ||||
| +        {
 | ||||
| +          g_set_error_literal (error,
 | ||||
| +                               G_IO_ERROR,
 | ||||
| +                               G_IO_ERROR_IS_DIRECTORY,
 | ||||
| +                               _("Target file is a directory"));
 | ||||
| +          goto err_out;
 | ||||
| +        }
 | ||||
| +      else if (!is_symlink ||
 | ||||
| +#ifdef S_ISLNK
 | ||||
| +               !S_ISLNK (original_stat.st_mode)
 | ||||
| +#else
 | ||||
| +               FALSE
 | ||||
| +#endif
 | ||||
| +               )
 | ||||
| +        {
 | ||||
| +          g_set_error_literal (error,
 | ||||
| +                               G_IO_ERROR,
 | ||||
| +                               G_IO_ERROR_NOT_REGULAR_FILE,
 | ||||
| +                               _("Target file is not a regular file"));
 | ||||
| +          goto err_out;
 | ||||
| +        }
 | ||||
|      } | ||||
|     | ||||
|    if (etag != NULL) | ||||
| @@ -1006,7 +1035,8 @@ handle_overwrite_open (const char    *filename,
 | ||||
|  	    } | ||||
|  	} | ||||
|   | ||||
| -      g_close (fd, NULL);
 | ||||
| +      if (fd >= 0)
 | ||||
| +        g_close (fd, NULL);
 | ||||
|        *temp_filename = tmp_filename; | ||||
|        return tmpfd; | ||||
|      } | ||||
| diff --git a/gio/tests/file.c b/gio/tests/file.c
 | ||||
| index efb2eaadd..bc55f3af4 100644
 | ||||
| --- a/gio/tests/file.c
 | ||||
| +++ b/gio/tests/file.c
 | ||||
| @@ -804,6 +804,113 @@ test_replace_cancel (void)
 | ||||
|    g_object_unref (tmpdir); | ||||
|  } | ||||
|   | ||||
| +static void
 | ||||
| +test_replace_symlink (void)
 | ||||
| +{
 | ||||
| +#ifdef G_OS_UNIX
 | ||||
| +  gchar *tmpdir_path = NULL;
 | ||||
| +  GFile *tmpdir = NULL, *source_file = NULL, *target_file = NULL;
 | ||||
| +  GFileOutputStream *stream = NULL;
 | ||||
| +  const gchar *new_contents = "this is a test message which should be written to source and not target";
 | ||||
| +  gsize n_written;
 | ||||
| +  GFileEnumerator *enumerator = NULL;
 | ||||
| +  GFileInfo *info = NULL;
 | ||||
| +  gchar *contents = NULL;
 | ||||
| +  gsize length = 0;
 | ||||
| +  GError *local_error = NULL;
 | ||||
| +
 | ||||
| +  g_test_bug ("https://gitlab.gnome.org/GNOME/glib/-/issues/2325");
 | ||||
| +  g_test_summary ("Test that G_FILE_CREATE_REPLACE_DESTINATION doesn’t follow symlinks");
 | ||||
| +
 | ||||
| +  /* Create a fresh, empty working directory. */
 | ||||
| +  tmpdir_path = g_dir_make_tmp ("g_file_replace_symlink_XXXXXX", &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  tmpdir = g_file_new_for_path (tmpdir_path);
 | ||||
| +
 | ||||
| +  g_test_message ("Using temporary directory %s", tmpdir_path);
 | ||||
| +  g_free (tmpdir_path);
 | ||||
| +
 | ||||
| +  /* Create symlink `source` which points to `target`. */
 | ||||
| +  source_file = g_file_get_child (tmpdir, "source");
 | ||||
| +  target_file = g_file_get_child (tmpdir, "target");
 | ||||
| +  g_file_make_symbolic_link (source_file, "target", NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  /* Ensure that `target` doesn’t exist */
 | ||||
| +  g_assert_false (g_file_query_exists (target_file, NULL));
 | ||||
| +
 | ||||
| +  /* Replace the `source` symlink with a regular file using
 | ||||
| +   * %G_FILE_CREATE_REPLACE_DESTINATION, which should replace it *without*
 | ||||
| +   * following the symlink */
 | ||||
| +  stream = g_file_replace (source_file, NULL, FALSE  /* no backup */,
 | ||||
| +                           G_FILE_CREATE_REPLACE_DESTINATION, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  g_output_stream_write_all (G_OUTPUT_STREAM (stream), new_contents, strlen (new_contents),
 | ||||
| +                             &n_written, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  g_assert_cmpint (n_written, ==, strlen (new_contents));
 | ||||
| +
 | ||||
| +  g_output_stream_close (G_OUTPUT_STREAM (stream), NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  g_clear_object (&stream);
 | ||||
| +
 | ||||
| +  /* At this point, there should still only be one file: `source`. It should
 | ||||
| +   * now be a regular file. `target` should not exist. */
 | ||||
| +  enumerator = g_file_enumerate_children (tmpdir,
 | ||||
| +                                          G_FILE_ATTRIBUTE_STANDARD_NAME ","
 | ||||
| +                                          G_FILE_ATTRIBUTE_STANDARD_TYPE,
 | ||||
| +                                          G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  info = g_file_enumerator_next_file (enumerator, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  g_assert_nonnull (info);
 | ||||
| +
 | ||||
| +  g_assert_cmpstr (g_file_info_get_name (info), ==, "source");
 | ||||
| +  g_assert_cmpint (g_file_info_get_file_type (info), ==, G_FILE_TYPE_REGULAR);
 | ||||
| +
 | ||||
| +  g_clear_object (&info);
 | ||||
| +
 | ||||
| +  info = g_file_enumerator_next_file (enumerator, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  g_assert_null (info);
 | ||||
| +
 | ||||
| +  g_file_enumerator_close (enumerator, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  g_clear_object (&enumerator);
 | ||||
| +
 | ||||
| +  /* Double-check that `target` doesn’t exist */
 | ||||
| +  g_assert_false (g_file_query_exists (target_file, NULL));
 | ||||
| +
 | ||||
| +  /* Check the content of `source`. */
 | ||||
| +  g_file_load_contents (source_file,
 | ||||
| +                        NULL,
 | ||||
| +                        &contents,
 | ||||
| +                        &length,
 | ||||
| +                        NULL,
 | ||||
| +                        &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +  g_assert_cmpstr (contents, ==, new_contents);
 | ||||
| +  g_assert_cmpuint (length, ==, strlen (new_contents));
 | ||||
| +  g_free (contents);
 | ||||
| +
 | ||||
| +  /* Tidy up. */
 | ||||
| +  g_file_delete (source_file, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  g_file_delete (tmpdir, NULL, &local_error);
 | ||||
| +  g_assert_no_error (local_error);
 | ||||
| +
 | ||||
| +  g_clear_object (&target_file);
 | ||||
| +  g_clear_object (&source_file);
 | ||||
| +  g_clear_object (&tmpdir);
 | ||||
| +#else  /* if !G_OS_UNIX */
 | ||||
| +  g_test_skip ("Symlink replacement tests can only be run on Unix")
 | ||||
| +#endif
 | ||||
| +}
 | ||||
| +
 | ||||
|  static void | ||||
|  on_file_deleted (GObject      *object, | ||||
|  		 GAsyncResult *result, | ||||
| @@ -1754,6 +1861,7 @@ main (int argc, char *argv[])
 | ||||
|    g_test_add_data_func ("/file/async-create-delete/4096", GINT_TO_POINTER (4096), test_create_delete); | ||||
|    g_test_add_func ("/file/replace-load", test_replace_load); | ||||
|    g_test_add_func ("/file/replace-cancel", test_replace_cancel); | ||||
| +  g_test_add_func ("/file/replace-symlink", test_replace_symlink);
 | ||||
|    g_test_add_func ("/file/async-delete", test_async_delete); | ||||
|  #ifdef G_OS_UNIX | ||||
|    g_test_add_func ("/file/copy-preserve-mode", test_copy_preserve_mode); | ||||
| -- 
 | ||||
| 2.30.1 | ||||
| 
 | ||||
		Reference in a new issue