Skip to content

cd-icc: set an error when a description/text tag is present but empty - #209

Merged
hughsie merged 2 commits into
hughsie:mainfrom
dal00:fix-mluc-empty-null-error
Jul 1, 2026
Merged

hughsie merged 2 commits into
hughsie:mainfrom
dal00:fix-mluc-empty-null-error

Conversation

@dal00

@dal00 dal00 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

cd_icc_get_mluc_data() can return NULL without setting the GError when a profile's text tag (e.g. desc) exists but contains a zero-length string. Callers assume "NULL return ⇒ error set", so this leads to a NULL-pointer dereference — and in the daemon, a crash.

Root cause

In lib/colord/cd-icc.c, the two cmsMLUgetWide() == 0 paths jump to out: (which does return value; with value still NULL) without calling g_set_error*. The sibling "tag missing" branch just above already sets CD_ICC_ERROR_NO_DATA, so these two paths were simply missing the same treatment.

This propagates through:

cd_icc_get_description()             lib/colord/cd-icc.c
  -> cd_profile_set_from_profile()   src/cd-profile.c:831   (value == NULL -> return FALSE)
    -> cd_profile_load_from_fd()     src/cd-profile.c:1005  (propagates FALSE, error still NULL)
      -> cd_main_daemon_method_call() src/cd-main.c:1494
             g_warning ("CdMain: failed to profile from fd: %s", error->message);

With error == NULL, error->message dereferences offset 8 of a NULL pointer → SIGSEGV. This is reachable over D-Bus via CreateProfile / CreateProfileWithFd whenever a client (e.g. gnome-settings-daemon / mutter registering a display) loads such a profile, so in practice it crashes colord on every login for affected users, leaving Settings → Color empty. (Several existing "colormgr get-devices is empty" reports share this symptom.)

The malformed profiles seen in the wild have a desc mluc tag with a single enUS record of length 0 — something lcms tolerates on read but never writes itself.

Fix

Set CD_ICC_ERROR_NO_DATA on the empty-text paths, mirroring the existing tag-missing branch, so the function never returns NULL with the error unset. This fixes all four MLU getters (description/copyright/manufacturer/model) and, transitively, the daemon crash.

Test

Adds /colord/icc{empty-desc} to cd-test-private.c with a small fixture (data/tests/empty-desc.icc: a normal sRGB profile whose desc mluc record length is forced to 0). It asserts cd_icc_get_description() returns NULL with CD_ICC_ERROR_NO_DATA. The test fails against master and passes with this change.

Verification

  • Unit: the new regression test fails without the fix, passes with it; the full colord-test-private suite passes (no regressions).
  • Live daemon (colord 1.4.8): with an affected profile, an unpatched daemon takes a SIGSEGV in cd_main_daemon_method_call on CreateProfileWithFd; the patched daemon survives, logging CdMain: failed to profile from fd: cmsSigProfile*Tag is empty and returning a proper D-Bus error.

dal00 added 2 commits July 1, 2026 16:41
cd_icc_get_mluc_data() returned NULL without setting the GError on the
two cmsMLUgetWide() == 0 paths -- a text tag that exists but holds a
zero-length string. Callers assume a NULL return implies the error is
set, so this NULL-with-no-error can be dereferenced: in the daemon,
cd_main_daemon_method_call() reads error->message after a failed
CreateProfileWithFd and crashes with SIGSEGV (reachable over D-Bus, e.g.
when a display's profile is registered at login).

Set CD_ICC_ERROR_NO_DATA on these paths, mirroring the existing
tag-missing branch just above, so the function never returns NULL with
the error unset. Fixes the description/copyright/manufacturer/model
getters and the resulting daemon crash.
Add /colord/icc{empty-desc}, backed by empty-desc.icc: a normal sRGB
profile whose 'desc' mluc record has a zero-length string. The test
asserts cd_icc_get_description() returns NULL with CD_ICC_ERROR_NO_DATA
rather than NULL with no GError set. It fails before the previous commit
and passes after it.
@hughsie
hughsie merged commit 37adffb into hughsie:main Jul 1, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants