0

I am testing a small issue with a daemon here (written in linux). I want to know whether what is done is right or not.

The daemon loads a shared object file (.so) using dlopen call. The the shared object receives some buffers from clients over the network. It uses the following call to read the buffer:

read_buffer(something, length of buffer read, buffer contents);

The read_buffer function copies the buffer of length specified in the second parameter, to another location using memcpy

On the client side, the following is done:

write_buffer(something, length of buffer, buffer contents);

The problem is that if we send an invalid length parameter (not matching the real length of what is copied in the third parameter), from the client side, there is a segfault in the server side in the memcpy location

I am not sure how to input-validate the parameters that are passed to memcpy function

Request you to please help me out understand what is possible solution

Deaf Ear
  • 153
  • 12
  • 1
    When you pass an invalid length, what is it? And why does it become invalid? Have you tried to step through the code in a debugger to see why it becomes invalid? – Some programmer dude Jun 18 '12 at 07:20
  • 1
    1) don't pass an invalid length. 2) are you ignoring the return value from read() and write() ? 3) Please post some real code. – wildplasser Jun 18 '12 at 07:34

2 Answers2

2

To check C/C++ code for errors in memory allocation / access, use Valgrind. The server side has no way (that I know of) to determine if the parmateres passed are valid or not. That's the C/C++ credo: Know what you're doing, or you die. There is no safety net.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • The invalid length is passed by an attack script with an intention to crash the server. – Deaf Ear Jun 18 '12 at 07:42
  • But you can compare the "invalid length" with the size of your buffer, before memcpy()ing, couldn't you ? – wildplasser Jun 18 '12 at 07:46
  • @DeafEar You really need to edit your question with this new, sudden information. If that is the reason why an invalid length would appear, you would likely need to use security measures such as encryption. – Lundin Jun 18 '12 at 07:48
  • I am sorry I didn't mention the attack scenario. Changing the summary – Deaf Ear Jun 18 '12 at 07:54
  • @wildplasser forgot to mention it is not a null terminated buffer. – Deaf Ear Jun 18 '12 at 07:55
  • @DeafEar: With that additional information, look at Manik Sidana's answer, which is most likely the way to go. – DevSolar Jun 18 '12 at 08:51
  • @DeafEar: I knew that. Network streams are (in principle) never nul-terminated. If you call read(), you should use not more than the allocated size of the buffer as the 3rd argument. If you want to read more, you should use a larger buffer. And if read() happens to return less than the 3rd argument, you should (repeatedly) call read again to get the remaining part, and append it to the previous parts. And you still did not show the relevant part of the code. – wildplasser Jun 18 '12 at 09:37
  • Thanks a lot all :) Manik's solution looks meaningful; @wildplasser I am a tester and I have only been given function names and a vague idea of what they do. So, I don't have code. I raised a security bug and wanted to understand what the dev guys are talking about :) Thanks for the knowledge sharing :) – Deaf Ear Jun 18 '12 at 11:01
1

You can append the length of the buffer at the start of the buffer.
-------------------------------------------------------------
| FIXED LENGTH OF BUFFER - n bytes | BUFFER
-------------------------------------------------------------

Now each time you read it on the server side, first read "n" bytes(reserved for storing the length) which contain the length. When the data arrives, you can compare length of buffer and first n bytes for validation. Hope this helps.

Manik Sidana
  • 2,005
  • 2
  • 18
  • 29