fix(psd): validate color_mode before RawColor early return#5282
Conversation
validate_header() let the oiio:RawColor/psd:RawData path return true before the switch that rejects unknown m_header.color_mode values, so an out-of-range or "hole" (undocumented) color mode reached setup(), which unconditionally indexes the fixed 10-entry mode_channel_count / mode_channel_names tables with the attacker-controlled value -- a global out-of-bounds read (and, per external report, a path to unbounded/bogus allocation) reachable via the public RawColor config attribute on a crafted PSD. Move the "is this a color mode we know about at all" check ahead of the RawColor early return so it always runs, and keep the existing Duotone/Lab-unsupported-without-raw check as a second pass after the early return, preserving current behavior for every valid mode. Assisted-by: Claude Code / claude-sonnet-5 Signed-off-by: Larry Gritz <lg@larrygritz.com>
| // return below: setup() indexes mode_channel_count/mode_channel_names by | ||
| // m_header.color_mode unconditionally (even for raw reads), so an | ||
| // out-of-table value (or one of the undocumented gaps, e.g. 5 and 6) | ||
| // must never reach that point regardless of m_WantRaw. |
There was a problem hiding this comment.
These long comments (from the recent PRs), sometimes longer than the code they guard, are a bit... much.
I think the first sentence of this is sufficient enough. i.e We're not going to put a comment on the conditional above this that says "Reject any depth outside our supported values before the RawColor early return below" etc.
I also personally think it's fine to put a bug number in the comment that hints at why things are the way they are: "Reject any color mode outside the known set before the RawColor early return below (see issue #5282)". That is much better than reading a novel and then having to git blame to find why the heck the long comment was added.
Maybe also put a comment on the m_WantRaw early return that says "Every invariant for raw images MUST be validated by this point"
There was a problem hiding this comment.
Yeah, I agree. Thanks for catching.
Signed-off-by: Larry Gritz <lg@larrygritz.com>
2 participants
validate_header() let the oiio:RawColor/psd:RawData path return true before the switch that rejects unknown m_header.color_mode values, so an out-of-range or "hole" (undocumented) color mode reached setup(), which unconditionally indexes the fixed 10-entry mode_channel_count / mode_channel_names tables with the attacker-controlled value -- a global out-of-bounds read (and, per external report, a path to unbounded/bogus allocation) reachable via the public RawColor config attribute on a crafted PSD.
Move the "is this a color mode we know about at all" check ahead of the RawColor early return so it always runs, and keep the existing Duotone/Lab-unsupported-without-raw check as a second pass after the early return, preserving current behavior for every valid mode.
Assisted-by: Claude Code / claude-sonnet-5