Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix overflows in outline processing #432

Merged
merged 1 commit into from Oct 8, 2020

Conversation

MrSmile
Copy link
Member

@MrSmile MrSmile commented Sep 27, 2020

I've added a couple of checks for overflows in outline pipeline. Should fix #431, but I'm not familiar with that fuzzing business so I can't ascertain it for now.

@TheOneric
Copy link
Member

Can confirm that this fixes #431.

Overflow checks seem fine to me. (Assuming abs(OUTLINE_MAX) < abs(OUTLINE_MIN) will stay true in future version, which it probably will, I guess)

@MrSmile
Copy link
Member Author

MrSmile commented Sep 28, 2020

Assuming abs(OUTLINE_MAX) < abs(OUTLINE_MIN) will stay true in future version, which it probably will, I guess

Maybe it's even better to get rid of OUTLINE_MIN altogether and use -OUTLINE_MAX instead.

@MrSmile
Copy link
Member Author

MrSmile commented Sep 29, 2020

New, more strict version. max(abs(x), abs(y)) <= OUTLINE_MAX is now enforced invariant of ASS_Outline instead of some ad hoc check. Also I've got rid of OUTLINE_MIN.

libass/ass_outline.c Outdated Show resolved Hide resolved
This commit enforces strict invariant on ASS_Outline
to contain point coordinates into predetermined range.

Fixes libass#431.
@astiob astiob merged commit 676f9dc into libass:master Oct 8, 2020
@astiob astiob added this to the 0.15.0 milestone Oct 8, 2020
@MrSmile MrSmile deleted the outline-overflow branch October 8, 2020 21:51
@astiob
Copy link
Member

astiob commented Oct 9, 2020

@MrSmile

ass_outline.c:53:12: warning: absolute value function 'abs' given an argument of type 'const FT_Pos' (aka 'const long') but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
    return abs(pt->x) <= OUTLINE_MAX && abs(pt->y) <= OUTLINE_MAX;
           ^
ass_outline.c:53:12: note: use function 'labs' instead
    return abs(pt->x) <= OUTLINE_MAX && abs(pt->y) <= OUTLINE_MAX;
           ^~~
           labs
ass_outline.c:53:41: warning: absolute value function 'abs' given an argument of type 'const FT_Pos' (aka 'const long') but has parameter of type 'int' which may cause truncation of value [-Wabsolute-value]
    return abs(pt->x) <= OUTLINE_MAX && abs(pt->y) <= OUTLINE_MAX;
                                        ^
ass_outline.c:53:41: note: use function 'labs' instead
    return abs(pt->x) <= OUTLINE_MAX && abs(pt->y) <= OUTLINE_MAX;
                                        ^~~
                                        labs

@MrSmile
Copy link
Member Author

MrSmile commented Oct 10, 2020

FT docs specify that FT_Pos is 16.16 or 26.6 fixed-point representation, so I'm not sure what's better, to cast into int32_t or use labs here.

@astiob
Copy link
Member

astiob commented Oct 10, 2020

Just use labs. In a way, this will also serve as a sanity check to ensure the FT docs aren’t lying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants