I've been seeing an issue in the schannel code where the internal encrypted buffer keeps growing over time during a large download. I went back and saw that there was some discussion on this topic a couple of years ago, and changes were made and then later reverted. What I'm seeing looks to me like a bug in the logic, though perhaps I'm missing something.

The schannel_recv function has this code to increase the encrypted buffer when needed (there's a similar bit of code in the handshake, but I'm not seeing an issue there):

  while(connssl->encdata_length - connssl->encdata_offset < CURL_SCHANNEL_BUFFER_FREE_SIZE || connssl->encdata_length < len) { /* increase internal encrypted data buffer */ }

My understanding is that the buffer is increased in size if either the existing buffer has less than CURL_SCHANNEL_BUFFER_FREE_SIZE (1024) bytes available in it, or if the entire existing buffer is too small to hold the requested amount of data. If either is true, the buffer is realloc'd twice its previous size.

The basic workflow in this method is:
1. Read data from the socket - enough to fill our encrypted buffer
2. While we have more encrypted data to process, and we haven't obtained enough decrypted data yet:
    a. Decrypt the data (as much as schannel will decrypt)
    b. Copy the decrypted data to the decrypted buffer
    c. Move any remaining encrypted data to the front of the encrypted buffer
3. Copy off the decrypted bytes to the caller's buffer
4. If there is any remaining decrypted bytes, move those to the front of the decrypted buffer (save them for the next call)

The issue seems to be that we can exit the while loop with the encrypted buffer nearly full. Then the next time the function is called, there is less than CURL_SCHANNEL_BUFFER_FREE_SIZE bytes available, and the buffer is reallocated. However, there's plenty of data in the encrypted buffer - more than enough to provide the caller with the number of bytes they requested. Let me give an example:

Say the encrypted buffer is already 32KB in size based on previous calls to this function, and the decrypted buffer contains 15.5KB of data left over from the previous call. This function is called again with a request for 16KB of data. We read enough data from the socket to fill up the encrypted buffer, and then decrypt. Say that the DecryptMessage function only returns 500 bytes of decrypted data, with the other 31.5KB left for the next decryption call. These 500 bytes, plus the 15.5KB in the decrypted buffer from last time, are enough to satisfy the caller's request, so we exit the while loop, leaving 31.5KB in the encrypted buffer. Then the next time the function is called, we are within the "free size" limit and the buffer gets reallocated. But this reallocation isn't necessary, as there's much more encrypted data available than what the caller is asking for (16KB). We should be able to read a small amount of data from the socket to fill up
 our decrypted buffer (500 bytes) and proceed as normal (or even skip the socket read completely since there's plenty of data already).

So I'm not clear what the "free size" check is intended to do - perhaps just make sure the buffer expands nicely from its original size? Once the buffer is larger than the len parameter, I don't believe expanding it further does any good - it just consumes extra memory. This scenario above can happen multiple times during a single download, and can result in the encrypted buffer doubling in size many times - I saw it get as high as 256 MB.
 
Thanks in advance.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2014-11-07