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

Email related examples use CURLOPT_READFUNCTION callback with bad coding practices #7330

Closed
piru opened this issue Jul 1, 2021 · 0 comments

Comments

@piru
Copy link

piru commented Jul 1, 2021

Problem description

The CURLOPT_READFUNCTION function payload_source used in many SMTP/IMAP examples depend on the specific implementation details of the libcurl. The payload_source function assumes that a single large enough call to the function will be made, and that the read is large enough to fit the whole line. This directly contradicts what CURLOPT_READFUNCTION API documentation says you should do: The data area pointed at by the pointer buffer should be filled up with at most size multiplied with nitems number of bytes by your function.

While libcurl might be calling the callback in a way that makes these examples work for now, if the call pattern ever changes the example apps may cease to work or even start crashing due to buffer overflow. Even if no such change happens, the example apps should try to be as correct as possible to not proliferate incorrect and/or unsafe API usage.

I expected the following

The example app implementing CURLOPT_READFUNCTION as documented: return maximum size * nmemb bytes and keep track of the "read" position.

curl/libcurl version

git HEAD

operating system

N/A

bagder added a commit that referenced this issue Jul 1, 2021
The same callback code is used in:

 imap-append.c
 smtp-authzid.c
 smtp-mail.c
 smtp-multi.c
 smtp-ssl.c
 smtp-tls.c

It should not assume that it can copy full lines into the buffer as it
will encourage sloppy coding practices. Instead use byte-wise logic and
check/acknowledge the buffer size appropriately.

Reported-by: Harry Sintonen
Fixes #7330
@bagder bagder closed this as completed in a37fc62 Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants