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

Jump to bottom

Avoid two unsigned int overflows in FTP listing parser #3225

Closed
wants to merge 1 commit into from
Closed

Avoid two unsigned int overflows in FTP listing parser #3225

wants to merge 1 commit into from

Conversation

rockdaboot
Copy link
Contributor

Curl_ftp_parselist: avoid unsigned integer overflows detected by clang

The overflow has no real world impact.
Just avoid it for "best practice".

Details
ftplistparser.c:

With any input and first char no being a digit (0-9), the execution
goes straight to L408. This results in the 'unsigned int overflow'.

Then the execution goes straight to L1007 which causes the second
'unsigned int overflow'.

A simple 'continue' in L408 instead of 'i--' avoids both warnings.

ftplistparser.c:408:14: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'unsigned long'
    #0 0x892d3d in Curl_ftp_parselist /home/tim/src/curl/lib/ftplistparser.c:408:14
    #1 0x667134 in chop_write /home/tim/src/curl/lib/sendf.c:585:22
    #2 0x6f73e3 in readwrite_data /home/tim/src/curl/lib/transfer.c:806:26
    #3 0x6f055a in Curl_readwrite /home/tim/src/curl/lib/transfer.c:1163:14
    #4 0x5581c8 in multi_runsingle /home/tim/src/curl/lib/multi.c:1912:16
    #5 0x54e98b in curl_multi_perform /home/tim/src/curl/lib/multi.c:2178:14
    #6 0x52997b in easy_transfer /home/tim/src/curl/lib/easy.c:686:15
    #7 0x521d0a in easy_perform /home/tim/src/curl/lib/easy.c:779:42

ftplistparser.c:1007:6: runtime error: unsigned integer overflow: 18446744073709551615 + 1 cannot be represented in type 'unsigned long'
    #0 0x88ead4 in Curl_ftp_parselist /home/tim/src/curl/lib/ftplistparser.c:1007:6
    #1 0x667134 in chop_write /home/tim/src/curl/lib/sendf.c:585:22
    #2 0x6f73e3 in readwrite_data /home/tim/src/curl/lib/transfer.c:806:26
    #3 0x6f055a in Curl_readwrite /home/tim/src/curl/lib/transfer.c:1163:14
    #4 0x5581c8 in multi_runsingle /home/tim/src/curl/lib/multi.c:1912:16
    #5 0x54e98b in curl_multi_perform /home/tim/src/curl/lib/multi.c:2178:14
    #6 0x52997b in easy_transfer /home/tim/src/curl/lib/easy.c:686:15
    #7 0x521d0a in easy_perform /home/tim/src/curl/lib/easy.c:779:42

@bagder
Copy link
Member

bagder commented Nov 6, 2018

Thanks. That looks like a correct change. Would you mind rebasing and force-pushing this again so that we can get it through the tests green?

@rockdaboot
Avoid two unsigned int overflows in FTP listing parser
542ea63 
Curl_ftp_parselist: avoid unsigned integer overflows

The overflow has no real world impact.
Just avoid it for "best practice".
@rockdaboot
Copy link
Contributor Author

Done

@bagder
Copy link
Member

bagder commented Nov 9, 2018

Thanks!

@bagder bagder closed this in c05d77e Nov 9, 2018
@rockdaboot rockdaboot deleted the tmp-ftp-unsigned-overflow branch November 10, 2018 16:01
@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rockdaboot @bagder