GitLab

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Backport !1976 “Use the right permissions for directory watching on Win32” to glib-2-66

See merge request !1990

Using FILE_GENERIC_READ | FILE_GENERIC_WRITE access rights for directory monitoring
might cause problems, as noted in gimp#4594.

ReadDirectoryChanges only needs FILE_LIST_DIRECTORY, so use that.

Fixes: gimp#4594

Backport !2325 “file-roller symlink attack” to glib-2-66

See merge request !1982

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

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

This clarifies the code a little. It introduces no functional changes.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Since a following commit is going to add a new test which references
Gitlab, so it’s best to move the URI bases inside the test cases.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Backport !1934 “gdbus: Reject attempts to set future connection or server flags” to glib-2-66

See merge request !1945

The GDBusConnectionFlags and GDBusServerFlags can affect how we carry
out authentication and authorization, either making it more or less
restrictive, so it's desirable to "fail closed" if a program is compiled
against a new version of GLib but run against an old version.
Signed-off-by: Simon McVittie <smcv@collabora.com>

Backport !1942 “gbytearray: Do not accept too large byte arrays” to glib-2-66

See merge request !1944

GByteArray uses guint for storing the length of the byte array, but it
also has a constructor (g_byte_array_new_take) that takes length as a
gsize. gsize may be larger than guint (64 bits for gsize vs 32 bits
for guint). It is possible to call the function with a value greater
than G_MAXUINT, which will result in silent length truncation. This
may happen as a result of unreffing GBytes into GByteArray, so rather
be loud about it.

(Test case tweaked by Philip Withnall.)

(Backport 2.66: Add #include gstrfuncsprivate.h in the test case for
`g_memdup2()`.)

Backport !1941 “gkeyfilesettingsbackend: Fix basename handling when group is unset” to glib-2-66

See merge request !1943

This tests the two recent commits.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

These should never have been allowed; they will result in precondition
failures from the `GKeyFile` later on in the code.

A test will be added for this shortly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Fix an effective regression in commit
7781a9cb, which happens when
`convert_path()` is called with a `key` which contains no slashes. In
that case, the `key` is entirely the `basename`.

Prior to commit 7781a9cb, the code worked through a fluke of `i == -1`
cancelling out with the various additions in the `g_memdup()` call, and
effectively resulting in `g_strdup (key)`.

Spotted by Guido Berhoerster.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

[2.66] Fix regressions in 2.66.6 where negative gssize indicates strlen()

See merge request !1933

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit df450131)

The intention here was to assert that the length of the password fits
in a gssize. Passwords more than half the size of virtual memory are
probably excessive.

Fixes: a8b204ff "gtlspassword: Forbid very long TLS passwords"
Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 61bb52ec)

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 63f37f8c)

Adding test coverage indicated that this was another bug in 0cc11f74.

Fixes: 0cc11f74 "giochannel: Forbid very long line terminator strings"
Resolves: #2323Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit 5dc8b001)

The inverted condition is an obvious error introduced by ecdf9140.

Fixes #2323

(cherry picked from commit a149bf2f)

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Backport !1926 “Add g_memdup2()” to glib-2-66

See merge request !1927

The public API `GIOChannel.line_term_len` is only a `guint`. Ensure that
nul-terminated strings passed to `g_io_channel_set_line_term()` can’t
exceed that length. Use `g_memdup2()` to avoid a warning (`g_memdup()`
is due to be deprecated), but not to avoid a bug, since it’s also
limited to `G_MAXUINT`.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

The public API `g_tls_password_set_value_full()` (and the vfunc it
invokes) can only accept a `gssize` length. Ensure that nul-terminated
strings passed to `g_tls_password_set_value()` can’t exceed that length.
Use `g_memdup2()` to avoid an overflow if they’re longer than
`G_MAXUINT` similarly.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

Don’t use an `int`, that’s potentially too small. In practical terms,
this is not a problem, since no socket address is going to be that big.

By making these changes we can use `g_memdup2()` without warnings,
though. Fewer warnings is good.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

Previously, the code in `convert_path()` could not handle keys longer
than `G_MAXINT`, and would overflow if that was exceeded.

Convert the code to use `gsize` and `g_memdup2()` throughout, and
change from identifying the position of the final slash in the string
using a signed offset `i`, to using a pointer to the character (and
`strrchr()`). This allows the slash to be at any position in a
`G_MAXSIZE`-long string, without sacrificing a bit of the offset for
indicating whether a slash was found.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

This allows it to handle strings up to length `G_MAXSIZE` — previously
it would overflow with such strings.

Update the several copies of it identically.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

Previously it was handled as a `gssize`, which meant that if the
`stop_chars` string was longer than `G_MAXSSIZE` there would be an
overflow.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

The members of `URL_COMPONENTS` (`winhttp_file->url`) are `DWORD`s, i.e.
32-bit unsigned integers. Adding to and multiplying them may cause them
to overflow the unsigned integer bounds, even if the result is passed to
`g_memdup2()` which accepts a `gsize`.

Cast the `URL_COMPONENTS` members to `gsize` first to ensure that the
arithmetic is done in terms of `gsize`s rather than unsigned integers.

Spotted by Sebastian Dröge.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()` or an existing `gsize`
variable), so that they use `g_memdup2()` instead.

In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it

In particular, this fixes an overflow within `g_bytes_new()`, identified
as GHSL-2021-045 by GHSL team member Kevin Backhouse.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Fixes: GHSL-2021-045
Helps: #2319

Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()`), so that they use
`g_memdup2()` instead.

In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

Convert all the call sites which use `g_memdup()`’s length argument
trivially (for example, by passing a `sizeof()`), so that they use
`g_memdup2()` instead.

In almost all of these cases the use of `g_memdup()` would not have
caused problems, but it will soon be deprecated, so best port away from
it.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: #2319

This will replace the existing `g_memdup()` function for use within
GLib. It has an unavoidable security flaw of taking its `byte_size`
argument as a `guint` rather than as a `gsize`. Most callers will
expect it to be a `gsize`, and may pass in large values which could
silently be truncated, resulting in an undersize allocation compared
to what the caller expects.

This could lead to a classic buffer overflow vulnerability for many
callers of `g_memdup()`.

`g_memdup2()`, in comparison, takes its `byte_size` as a `gsize`.

Spotted by Kevin Backhouse of GHSL.

In GLib 2.68, `g_memdup2()` will be a new public API. In this version
for backport to older stable releases, it’s a new `static inline` API
in a private header, so that use of `g_memdup()` within GLib can be
fixed without adding a new API in a stable release series.
Signed-off-by: Philip Withnall <pwithnall@endlessos.org>
Helps: GHSL-2021-045
Helps: #2319

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Backport !1920 “Resolve GDBus regressions in setcap/setgid programs” to glib-2-66

See merge request !1922

This is against my better judgement, but it's the least bad regression
fix I can think of. If we don't do this, at least gnome-keyring-daemon
(setcap) and msmtp (setgid) are known to regress.

Resolves: #2305
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=981420
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=981555Signed-off-by: Simon McVittie <smcv@collabora.com>