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

An integer overflow found in src/tool_cb_prg.c #3456

Closed
lipeng28 opened this issue Jan 10, 2019 · 3 comments

Comments

Projects
None yet
2 participants
@lipeng28
Copy link

commented Jan 10, 2019

Hi There

Peng Li at Baidu X-Lab found an integer overflow in the function tool_progress_cb of src/tool_cb_prg.c, the error message is "tool_cb_prg.c:122:29: runtime error: signed integer overflow: 828 + 9223372036854775807 cannot be represented in type 'long'". Since the overflowed variable total is used in the consequent conditionals, I think it is a critical bug.

You could compile curl with undefined behavior sanitizer activated and apply "curl-ubsan -q -K id_005898" to reproduce the bug, the OS is ubuntu 16.04.2 LTS, curl's version is curl 7.61.1 (x86_64-pc-linux-gnu) libcurl/7.61.1. If you need any assistance, please let me know.

Thanks
Peng

int tool_progress_cb(void clientp,
curl_off_t dltotal, curl_off_t dlnow,
curl_off_t ultotal, curl_off_t ulnow)
{
/
The original progress-bar source code was written for curl by Lars Aas,
and this new edition inherits some of his concepts. */

struct timeval now = tvnow();
struct ProgressData *bar = (struct ProgressData *)clientp;
curl_off_t total;
curl_off_t point;

/* expected transfer size */
total = dltotal + ultotal + bar->initial_size;
...
}

id_005898.txt

@bagder bagder added the cmdline tool label Jan 10, 2019

@bagder

This comment has been minimized.

Copy link
Member

commented Jan 10, 2019

I think it is a critical bug.

Why? What else does this cause than making the progress bar not appear the way it should?

bagder added a commit that referenced this issue Jan 10, 2019

tool_cb_prg: avoid integer overflow
When calculating the progress bar width.

Reported-by: Peng Li
Fixes #3456
@lipeng28

This comment has been minimized.

Copy link
Author

commented Jan 10, 2019

Thank you bagder for your prompt fix!

@bagder bagder closed this in 61faa0b Jan 11, 2019

@lipeng28

This comment has been minimized.

Copy link
Author

commented Jan 15, 2019

Hi There

I confirmed that this fix resolves the problem, thank you for fixing them!

Best,
Peng

@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.