Created attachment 136224 [details] [review]
[PATCH 1/2] Make cd_color_get_blackbody_rgb_full safer

Validate arguments. If temp is divisible by 100, avoid interpolation
because it accesses beyond the data for temp == 10000.

Created attachment 136226 [details] [review]
[PATCH 2/2] Avoid buffer overflow when reading profile_id

The profile ID is 16 bytes, not 4 bytes. Use the union type specified by
the LCMS API.

Created attachment 136227 [details] [review]
[PATCH 2/2] Avoid buffer overflow when reading profile_id

Second version formatting the profile ID bytes individually to avoid endianness issues.

I've rewrote the 2nd patch, to this:

commit 2c92e03775a15bcd304ef39e9a3220496fc9168a (HEAD -> master)
Author: Richard Hughes <richard@hughsie.com>
Date:   Sun Dec 17 13:17:27 2017 +0000

    Avoid buffer overflow when reading profile_id
    
    Based on a patch by Jan Alexander Steffens, many thanks.
    
    Fixes half of: https://bugs.freedesktop.org/show_bug.cgi?id=104294

:100644 100644 99fa27b... 5e9441a... M  lib/colord/cd-icc.c

-- this now matches the MD5 that the daemon exposes.

The first patch also now gives me:

../lib/colord/cd-color.c:1471:11: warning: function call has aggregate value [-Waggregate-return]

... which makes it fail CI.

Is -Waggregate-return really a useful warning?

But OK, I can rewrite the patch not to use div() again.

Created attachment 136239 [details] [review]
[PATCH] Make cd_color_get_blackbody_rgb_full safer

Use / and % instead of div().

commit ba4bdb6f0dadc8f2d51122bfdee9a0325302f171
Author:     Jan Alexander Steffens (heftig) <jan.steffens@gmail.com>
AuthorDate: 2017-12-16 04:18:01 +0100
Commit:     Richard Hughes <richard@hughsie.com>
CommitDate: 2018-03-27 10:26:33 +0100

    Make cd_color_get_blackbody_rgb_full safer
    
    Validate arguments:
      - temp == NaN would fail the bounds checks and could result in
        out-of-bounds reads.
      - result == NULL is an obvious error.
    
    Avoid calling cd_color_rgb_interpolate if the second point would be
    outside the data array. This only happens for temp == 10000, which makes
    the alpha 0, so one would think that the garbage read has no effect on
    the result. However, if the garbage happens to contain NaNs they would
    propagate to the output. Besides, asan and valgrind still complain.