Skip to content

tool_help: add checks to avoid unsigned wrap around#19377

Closed
x2018 wants to merge 3 commits into
curl:masterfrom
x2018:tool_help_wrap_around
Closed

tool_help: add checks to avoid unsigned wrap around#19377
x2018 wants to merge 3 commits into
curl:masterfrom
x2018:tool_help_wrap_around

Conversation

@x2018

@x2018 x2018 commented Nov 5, 2025

Copy link
Copy Markdown
Contributor

get_terminal_columns() can return cols from 1-10000 (21-10000 only via env COLUMNS), while the longdesc in option --help all is 47.
When cols < 47, the unsigned subtraction cols - longdesc wraps around to a large value, which is then passed as negative width to curl_mprintf after conversion to int.
Therefore, it is better to properly check the unsigned boundary to avoid unsigned wrap around and any other unexpected/undefined behavior.

@bagder bagder left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It can be noted that curl's printf() implementation treats negative values just as it does zeros. I still agree with the fix.

@bagder bagder closed this in 69622ff Nov 6, 2025
@x2018

x2018 commented Nov 6, 2025

Copy link
Copy Markdown
Contributor Author

It can be noted that curl's printf() implementation treats negative values just as it does zeros. I still agree with the fix.

Thanks! It would not crash or something dangerous but it will really affect the display width according to https://github.com/curl/curl/blob/master/lib/mprintf.c#L1015.

For example as below:

@test:~/curl-8.17.0/build/src# COLUMNS=47 ./curl --help all | head -n 3
     --abstract-unix-socket <path>  Connect via abstract Unix domain socket
     --alt-svc <filename>  Enable alt-svc with this cache file
     --anyauth  Pick any authentication method
@test:~/curl-8.17.0/build/src# COLUMNS=65 ./curl --help all | head -n 3
     --abstract-unix-socket <path>  Connect via abstract Unix domain socket
     --alt-svc <filename>  Enable alt-svc with this cache file
     --anyauth       Pick any authentication method
@test:~/curl-8.17.0/build/src# COLUMNS=29 ./curl --help all | head -n 3
     --abstract-unix-socket <path>  Connect via abstract Unix domain socket
     --alt-svc <filename>  Enable alt-svc with this cache file
     --anyauth       Pick any authentication method

When COLUMNS=29, the width will be the same as COLUMNS=65 because of the wrap around (47-18=29 and 47+18=65).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants