Backport CVE-2021-27219 integer overflow fix to GLib 2.58

Debian 10 contains GLib 2.58 and is supported for a bit more than 1 more year, so I need to backport the CVE-2021-27219 integer overflow fixes to that version. Rather than doing this downstream with no review, I'd like to merge them into the GLib 2.58 branch to make them reviewable and available for other distros' benefit. @ebassi said this had been done in the past and seemed OK.

I do not intend this to imply any particular upstream support for GLib 2.58, and in particular I don't plan to make any new GLib 2.58.x releases.

Commits included

• gstrfuncs: Add internal g_memdup2() function

  From: @pwithnall

  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: @pwithnall
  Helps: CVE-2021-27219
  Helps: GHSL-2021-045
  Helps: #2319 (closed)
  (cherry picked from commit 5e5f75a7)

• glib: Use g_memdup2() instead of g_memdup() in obvious places

  From: @pwithnall

  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 (aka CVE-2021-27219) by GHSL team member Kevin Backhouse.

  Adapted for GLib 2.58 by Simon McVittie.

  Signed-off-by: @pwithnall
  Fixes: CVE-2021-27219
  Fixes: GHSL-2021-045
  Helps: #2319 (closed)
  (cherry picked from commit 0736b7c1)
  [Backport to 2.58: Omit changes to ghash.c, will be a separate commit]
  [Backport to 2.58: Omit changes to giochannel.c, not needed in this branch]
  [Backport to 2.58: Omit changes to uri test, not needed in this branch]
  Signed-off-by: @smcv

• ghash: Use g_memdup2() instead of g_memdup()

  New commit (sort of)

  Backport of part of commit 0736b7c1 to the simpler structure of the GHashTable code in glib-2-58.

  Helps: #2319 (closed)

• gobject: Use g_memdup2() instead of g_memdup() in obvious places

  From: @pwithnall

  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: @pwithnall
  Helps: #2319 (closed)
  (cherry picked from commit 6110caea)

• gio: Use g_memdup2() instead of g_memdup() in obvious places

  From: @pwithnall

  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: @pwithnall
  Helps: #2319 (closed)
  (cherry picked from commit be883434)

• gvariant test: Use g_memdup2

  New commit

  This code no longer existed on the glib-2-66 branch, but it's present in glib-2-58. It's easier to verify that all potentially problematic g_memdup() uses have been replaced if we replace these too.

  Helps: #2319 (closed)

• gwinhttpfile: Avoid arithmetic overflow when calculating a size

  From: @pwithnall

  The members of URL_COMPONENTS (winhttp_file->url) are DWORDs, 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 gsizes rather than unsigned integers.

  Spotted by Sebastian Dröge.

  Signed-off-by: @pwithnall
  Helps: #2319 (closed)
  (cherry picked from commit 0cbad673)

• gwin32: Use gsize internally in g_wcsdup()

  From: @pwithnall

  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.

  Adapted for GLib 2.58 by Simon McVittie.

  Signed-off-by: @pwithnall
  Helps: #2319 (closed)
  [Backport to 2.58 branch: g_wcsdup() existed in different places]
  Signed-off-by: @smcv

Commits not included here

If people feel strongly about any of these needing to be fixed as well, then I think they should be done as separate MRs that bundle the overflow-avoidance with the regression fixes (either in a group of commits or one squashed commit), because all of these were non-trivial code, and all except GSocket came with regressions.

• ba8ca443 in gio/gkeyfilesettingsbackend.c, which had regressions that had to be fixed later. I think the potential overflow here can only be a problem if a GSettings path is more than 2GB, but I think the regression risk of non-obvious fixes here is greater than the risk that someone is processing attacker-controlled GSettings paths.

• 65ec7f4d in GSocket, which is non-trivial. This can only overflow if there is a platform where the fixed-size struct sockaddr_storage is more than 2GB; I'm pretty sure that is not the case.

• 777b95a8 in GTlsPassword, which is non-trivial and initially had an inverted assertion that had to be fixed later. This can only be a problematic overflow if a program accepts TLS passwords greater than 2GB long, and is at a security boundary such that arbitrary code execution by the user choosing the TLS password would be unacceptable - it seems unlikely that those conditions exist in the same program. If we want to backport this one then we should also backport the superficial test coverage that I added (which can be done cleanly).

• ecdf9140 in GIOChannel, which had regressions that had to be fixed later. This can only be a problematic overflow if a program at a security boundary parses a stream with an attacker-controlled line delimiter, which seems unlikely. Its test coverage is not straightforward to backport, because I added it to a test-case that didn't exist in 2.58 and was designed to exercise fixes that 2.58 doesn't have.