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

Avoid two unsigned int overflows in FTP listing parser #3225

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@rockdaboot
Contributor

rockdaboot commented Nov 3, 2018

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

This comment has been minimized.

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?

Avoid two unsigned int overflows in FTP listing parser
Curl_ftp_parselist: avoid unsigned integer overflows

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

@rockdaboot rockdaboot force-pushed the rockdaboot:tmp-ftp-unsigned-overflow branch from 7696e58 to 542ea63 Nov 7, 2018

@rockdaboot

This comment has been minimized.

Contributor

rockdaboot commented Nov 7, 2018

Done

@bagder

This comment has been minimized.

Member

bagder commented Nov 9, 2018

Thanks!

@bagder bagder closed this in c05d77e Nov 9, 2018

@rockdaboot rockdaboot deleted the rockdaboot:tmp-ftp-unsigned-overflow branch Nov 10, 2018

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