Skip to content

sixel: prevent allocating an absurd amount of memory or writing OOB#20213

Merged
DHowett merged 1 commit into
mainfrom
dev/duhowett/s1x3l
May 12, 2026
Merged

sixel: prevent allocating an absurd amount of memory or writing OOB#20213
DHowett merged 1 commit into
mainfrom
dev/duhowett/s1x3l

Conversation

@DHowett
Copy link
Copy Markdown
Member

@DHowett DHowett commented May 11, 2026

This commit implements two fixes for the integer overflow/out-of-bounds write reported in #20149.

First, it catches any exception generated in sixel char processing (which will prevent out_of_memory or bad_alloc from being ignored sight-unseen, and will prevent the consumption of further DCS content).

Second, it prevents us from allocating memory for an image which will never be displayed (because it exceeds the height of the display.)

This supersedes prior work in #20153 for the same issues.

Closes #20149
Closes #20153

This commit implements two fixes for the integer overflow/out-of-bounds
write reported in #20149. First, it catches any exception generated in
sixel char processing (which will prevent `out_of_memory` or `bad_alloc`
from being ignored sight-unseen, and will prevent the consumption of
further DCS content). Second, it prevents us from allocating memory for
an image which will never be displayed (because it exceeds the height of
the display.)

This supersedes prior work in #20153 for the same issues.

Closes #20149
Closes #20153

Co-authored-by: James Holderness <j4_james@hotmail.com>
@DHowett DHowett requested review from j4james and lhecker May 11, 2026 23:01
@DHowett
Copy link
Copy Markdown
Member Author

DHowett commented May 11, 2026

My only concern is:

is there a potential conformance issue with that fix? If we get a new line we can't fit but don't advance the image cursor, we're going to overwrite the bottom-most line with the incoming data. I don't know how prevalent that is likely to be in the wild...

_parameters.clear();
return [&](const auto ch) {
_parseCommandChar(ch);
try
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot quantify the cost of an exception handler here.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not too concerned about the performance, and if we get Leonard's DCS batch processing merged at some point, I expect that would mitigate the performance impact (assuming there is any).

_availablePixelHeight -= _sixelHeight;
_resizeImageBuffer(_sixelHeight);
_fillImageBackgroundWhenScrolled();
// If we don't have any available pixel height, that means the image has
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(this is getting a Co-authored-by; github is just suppressing it in the PR review window)

Copy link
Copy Markdown
Collaborator

@j4james j4james left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DHowett DHowett enabled auto-merge (squash) May 12, 2026 15:00
@DHowett DHowett disabled auto-merge May 12, 2026 15:01
@DHowett DHowett enabled auto-merge (squash) May 12, 2026 15:01
@DHowett DHowett merged commit c829d4c into main May 12, 2026
20 checks passed
@DHowett DHowett deleted the dev/duhowett/s1x3l branch May 12, 2026 16:15
DHowett added a commit that referenced this pull request May 12, 2026
…20213)

This commit implements two fixes for the integer overflow/out-of-bounds
write reported in #20149.

First, it catches any exception generated in sixel char processing
(which will prevent `out_of_memory` or `bad_alloc` from being ignored
sight-unseen, and will prevent the consumption of further DCS content).

Second, it prevents us from allocating memory for an image which will
never be displayed (because it exceeds the height of the display.)

This supersedes prior work in #20153 for the same issues.

Closes #20149
Closes #20153

Co-authored-by: James Holderness <j4_james@hotmail.com>
(cherry picked from commit c829d4c)
Service-Card-Id: PVTI_lADOAF3p4s4BBcTlzgrY_zM
Service-Version: 1.24
DHowett added a commit that referenced this pull request May 12, 2026
…20213)

This commit implements two fixes for the integer overflow/out-of-bounds
write reported in #20149.

First, it catches any exception generated in sixel char processing
(which will prevent `out_of_memory` or `bad_alloc` from being ignored
sight-unseen, and will prevent the consumption of further DCS content).

Second, it prevents us from allocating memory for an image which will
never be displayed (because it exceeds the height of the display.)

This supersedes prior work in #20153 for the same issues.

Closes #20149
Closes #20153

Co-authored-by: James Holderness <j4_james@hotmail.com>
(cherry picked from commit c829d4c)
Service-Card-Id: PVTI_lADOAF3p4s4BQX0-zgrY_zQ
Service-Version: 1.25
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.

[SECURITY] Potential Out-of-Bounds Write in Windows Terminal OpenConsole SIXEL Parser via Integer Overflow

3 participants