cURL cURL > Mailing List > Monthly Index > Single Mail

curl-tracker Archives

[curl:bugs] #1450 Schannel internal buffers grow without limit during large download

From: Warren Menzer <wmenzer_at_users.sf.net>
Date: Mon, 15 Dec 2014 21:47:37 +0000

Thanks, Marc. It's going to be a few days before I have a chance to try this patch in my code, but looking at it, I'm not sure if it completely solves the issue or not. The if statement (which is no longer a while loop) still checks:

connssl->encdata_length - connssl->encdata_offset < CURL_SCHANNEL_BUFFER_FREE_SIZE

I'm not sure if that's really a valid check or not. In my example above, the previous call to this method left the encrypted buffer nearly full (with less than CURL_SCHANNEL_BUFFER_FREE_SIZE bytes free in it), but depending on the length requested (len), that doesn't necessarily mean a larger buffer is needed. So while it won't double the size of the buffer each time that condition is met (which is good), it's still going to reallocate, even if there is way more encrypted data to decrypt than the caller needs.

Does that make sense? It looks like this will get rid of the creeping memory usage - I just wonder if we'll be needlessly reallocating sometimes. It shouldn't be too common, but it would happen any time the encrypted buffer happened to be left nearly full the last time through.

---
** [bugs:#1450] Schannel internal buffers grow without limit during large download**
**Status:** open
**Created:** Mon Nov 10, 2014 03:51 PM UTC by Warren Menzer
**Last Updated:** Sun Dec 14, 2014 05:03 PM UTC
**Owner:** nobody
libcurl version: 7.39.0
Operating System: Windows 7
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: 
    * Decrypt the data (as much as schannel will decrypt) 
    * Copy the decrypted data to the decrypted buffer 
    * 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. 
---
Sent from sourceforge.net because curl-tracker@cool.haxx.se is subscribed to https://sourceforge.net/p/curl/bugs/
To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/curl/admin/bugs/options.  Or, if this is a mailing list, you can unsubscribe from the mailing list.
Received on 2014-12-15

These mail archives are generated by hypermail.