Thread: What are the vulnerabilities associated with this code?

  1. #1
    Registered User
    Join Date
    May 2012
    Posts
    12

    What are the vulnerabilities associated with this code?

    Hi guys,
    I'm stuck with what kind of attack could occur to the code posted below, would it be a buffer overflow or integer overflow?
    So what type of vulnerability? and if any one has any remedies that could be put in place to fix such code.


    Code:
    int grab_request (char buf[], int buf_len)
    {
    int i;
       for (i=0; i< buf_len; i++) {
           /* put things into buf */
       }
    buf[i] = ‘\0’;
    return i;
    }

  2. #2
    Registered User
    Join Date
    May 2010
    Posts
    4,632
    If your buf[] size is buf_len you will be accessing your array out of bounds when you try to insert the end of string character into the string on line 7. Because when you reach this point i will be equal to buf_len.

    Jim

  3. #3
    Registered User
    Join Date
    May 2012
    Posts
    12
    Ok,

    So the buf_length should be declared with a value, and that value should be less than the size of a CHAR.....because the char buf[] isn't given a value.....?
    Otherwise when it comes to line 7, the value of i may be far larger than that of the array.......?

    I hope what i have said just then does make some sense....otherwise i will feel extremely retarded. I already do.....by posting on here.

    What type of vulnerability is this then.......bufferoverflow?

  4. #4
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    This sounds like homework, so instead of giving you a direct answer, I will (hopefully) guide you to an answer. Do you see a potential for integer overflow? Think about "classic" test cases. Analyze what would happen if buf_len were 0, 1, -1, INT_MIN, INT_MAX, and a couple "normal" values, like 10. Is it ever possible to get i to roll over to a negative number? Is it possible that i reaches a value greater than buf_len and thus causes you to write past the end of buf? What about after the loop terminates?

  5. #5
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by scooter View Post
    Ok,

    So the buf_length should be declared with a value, and that value should be less than the size of a CHAR.....because the char buf[] isn't given a value.....?
    Otherwise when it comes to line 7, the value of i may be far larger than that of the array.......?

    I hope what i have said just then does make some sense....otherwise i will feel extremely retarded. I already do.....by posting on here.

    What type of vulnerability is this then.......bufferoverflow?
    For starters, it is impossible to tell for sure whether your function has bugs, without knowing what code is in the loop. Your loop may be safe for using buf[i], but what if some code in there accesses buf[i+1] or buf[i-1]?

    According to the C standard, the size of a char is 1, so it would make no sense to require buf_len to be less than that. A reasonable assumption is that buf_len contains the size of the buf parameter, i.e. how long the array is. The buf_len parameter is passed in to the function, so whoever uses your function must pass in the correct value. They must know that your function "adds" a byte at the end (the null character). So a typical call to your function might look like:
    Code:
    char mybuf[10];
    grab_request(mybuf, sizeof(mybuf));  // sizeof(mybuf) is 100, there are 100 elements in your array
    Now, run through your function by hand. Draw out an array with 10 spaces. That is buf in your function. In your loop, assign letters to each spot (buf[i] for each i). Just run through the alphabet, from 'A', 'B', etc until your loop stops. Keep track of i at each step. When your loop is done, what value does i have? What does the line buf[i] = '\0' do? Where does it write? Is that safe?

  6. #6
    Registered User
    Join Date
    May 2012
    Posts
    12
    Ok, well i appreciate the home schooling. Yes it is homework or an assignment. I'm all new....so go easy. I'm not here for the thread to carry out over dayz. This was my last option coming here.

    Ok, so when you say assign letters etc.....The array is 10 or the length of the buffer is 10. Which means....0,1,2,3,4,5,6,7,8,9 ......right?
    From what I comprehended in your advice....i finishes with the value of 9. And I presume the buffer (buf[i]) would contain the letter j??????
    Where does it write....????

    What is the '\0' do?

  7. #7
    Registered User
    Join Date
    May 2012
    Posts
    12
    BTW many thanks......I appreciate the help!

    I can see that this be a inter overflow.....i just dont know how it will occur.....

    Signed integers......mean that 0x7FFFFFFF -> 32 bit large positive number Adding 1 produces 0x80000000, a large negative number

    I just cant see it occuring....like we learnt about it...but honestly....i can't see it.

  8. #8
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    I don't want it to drag on for days either . But I do want you to actually learn, and I think that can happen in much less than a day. I'm trying to get you to try this out on paper and go through the code by hand. Pretend you are the computer interpreting the programming instructions in your function. That is how you analyze code. It is a skill you must develop, it is critical to any programming work you ever have to do. First, a quick announcement:

    The comments in my code from post #5 say 100. That is incorrect, they should say 10.

    Now, to your questions:

    > What is the '\0' do?
    It looks like it is supposed to be a "null terminator". A string in C is just a bunch of sequential bytes that ends with a zero byte, or null terminator. All a character is in C is a tiny (8-bit/1-byte) integer. The character '\0' is a character (byte) with a numeric value 0. In the context of strings in C, it marks the end of the string. So it appears that this function attempts to store a string of some sort in buf. That's why I picked letters to store, because strings usually contain human-readable data.

    > Ok, so when you say assign letters etc.....The array is 10 or the length of the buffer is 10. Which means....0,1,2,3,4,5,6,7,8,9 ......right?
    I declared the array to have 10 elements. The valid indexes (numbers I can put inside the [ ]) are 0 through 9, so you are correct there. You can say the array has 10 elements, or the buffer has length 10, they both mean the same thing. If you ever try to do mybuf[-1] or mybuf[10], that is a buffer "overflow" or "overrun" (some people call negative indexes "underflows" or "underruns").

    > From what I comprehended in your advice....i finishes with the value of 9. And I presume the buffer (buf[i]) would contain the letter j??????
    You're half right. i doesn't "finish" with the value 9, but buf[9] would contain a 'j'. Here's one way to make buf contain the alphabet*:
    Code:
    int grab_request (char buf[], int buf_len)
    {
    int i;
       for (i=0; i< buf_len; i++) {
           buf[i] = 'a' + i;  // this will put 'a' in buf[0], 'b' in buf[1], 'c' in buf[2], etc
    
       }
    buf[i] = ‘\0’;
    return i;
    }
    So a common way to analyze and step through code by hand is to make a table where each column is a variable or expression from your code, and each row represents the state of all the variables after each instruction in the code is processed.
    Code:
    statement executed       |   i    |       buf           | buf_len | i< buf_len 
    -------------------------+--------+---------------------+---------+------------
    start of function        |   ?    |        ?            |   10    |     ?      
    line 4: i=0;             |   0    |        ?            |   10    |  1 (true)  
    line 4: i< buf_len       |   0    |        ?            |   10    |  1 (true)  
    line 5: buf[0] = 'a' + 0 |   0    | 'a', ?, ?, ...      |   10    |  1 (true)  
    back to top of loop      |        |                     |         |            
    line 4: i++              |   1    | 'a', ?, ?, ...      |   10    |  1 (true)  
    line 4: i< buf_len       |   1    | 'a', ?, ?, ...      |   10    |  1 (true)  
    line 5: buf[1] = 'a' + 1 |   1    | 'a', 'b', ?, ?, ... |   10    |  1 (true)  
    back to top of loop      |        |                     |         |            
    line 4: i++              |   2    | 'a', 'b', ?, ?, ... |   10    |  1 (true)  
    line 4: i< buf_len       |   2    | 'a', 'b', ?, ?, ... |   10    |  1 (true)  
    line 5: buf[1] = 'a' + 1 |   2    | 'a', 'b', 'c', ?,...|   10    |  1 (true)  
    back to top of loop      |        |                     |         |
    The question marks represent unknown values, usually from variables that haven't been initialized yet. Remember, when you pass an array, the original array (mybuf), and the parameter you pass it in to (buf) are the same array, they refer to the same exact place in memory. So if you change buf in your function, you change mybuf too. Initially, all 10 spots in mybuf are uninitialized, therefore buf is also uninitialized. As we go through our loop, we start putting known values into the spots in buf, hence we know 'a', 'b', 'c', etc as we keep repeating our loop.

    Remember, buf[9] is the last spot in the array, buf[10] is wrong. Also remember, your for loop will continue until the i< buf_len part of your for loop is checked, and it is 0 (false). Only after that will it stop.

    Just keep on filling in the rows of that table until you get to the return i; line of code. Once you do this a few times, it becomes much easier and you can do it in your head. Then this stuff goes much quicker. Hopefully that clears up what you are supposed to be doing here and helps you figure out what vulnerabilities there might be and how to fix them.

    As for analyzing integer overflow, the process is similar. Pick some test values for buf_len (like those I mentioned in post #4) and go through your table. Does i ever exceed the maximum integer value? Note, you can pretend that INT_MAX is something small like 5 and INT_MIN is -5, so you don't have billions of lines in your table. You just want to be sure that, for any possible value of buf_len, i never goes beyond 5 in any way, that it never "makes it to 6", which would signify an overflow.

    I hope that's clear and helpful -- my brain is starting to shut down. It's getting late here, I worked a 14 hour day and have laundry to finish. Not sure if I'll check this again before morning, but that should give you plenty to chew on.

    * This only works with ASCII or compatible character sets (where the letters of the alphabet are sequential), and could result in problems if buf_len was large (more than 26), but we wont worry about that for now.

  9. #9
    Registered User
    Join Date
    Nov 2010
    Location
    Long Beach, CA
    Posts
    5,909
    Quote Originally Posted by scooter View Post
    BTW many thanks......I appreciate the help!

    I can see that this be a inter overflow.....i just dont know how it will occur.....

    Signed integers......mean that 0x7FFFFFFF -> 32 bit large positive number Adding 1 produces 0x80000000, a large negative number

    I just cant see it occuring....like we learnt about it...but honestly....i can't see it.
    You are right, that is exactly what an integer overflow is. You seem to be going off intuition. Think about why it can't happen. Some questions to ponder/answer:
    Assume INT_MAX is the largest possible integer. Pretend it's 5 for simplicity's sake.
    What if buf_len is negative? Does the loop ever execute? Does i ever increment beyond 0?
    What if buf_len is zero? Does the loop ever execute? Does i ever increment beyond 0?
    What if buf_len is 3? What is i when the loop stops? Does i ever increment beyond INT_MAX?
    What if buf_len is INT_MAX (the largest possible signed integer)? What is i when the loop stops? Does i ever increment beyond INT_MAX?

    Your intuition seems correct.

  10. #10
    Registered User
    Join Date
    May 2012
    Posts
    12
    So if buf_length were to be less than zero i.e. -1 then the i from the for loop would continue until it reaches the largest negative number which is one more than MAX_INT.
    Only then would the loop stop......and hence it would be an underflow/overflow of incorrect data.

    If buf_length would be 0 then i would not loop.....correct? because null cant be less than null? i.e. false statement.
    If buf_length were to be MAX_INT....then i would be one less than that correct...?

    What could be used to make a remedy...? Safeint class?

    thankyou for helping me with this overflow/underflow.....i appreciate it

  11. #11
    Registered User
    Join Date
    May 2012
    Posts
    12
    My lecture says this is not an integer overflow......more along the lines of what Jim says. If your buf[] size is buf_len you will be accessing your array out of bounds when you try to insert the end of string character into the string on line 7. Because when you reach this point i will be equal to buf_len.

    so what exactly is the vulernability......I really have no idea.

  12. #12
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    so what exactly is the vulernability......I really have no idea.
    It sounds like you have an idea.

    Why do you think you don't have an idea? Or really, what aren't you getting?

    The post by anduril462 was very good. Have you read it thoroughly?

    You should probably realize that "Your intuition seems correct." was probably aimed at "I just cant see it occuring.".

    Soma

  13. #13
    Registered User
    Join Date
    May 2012
    Posts
    12
    everyone is having a nice old laugh watching me solve this riddle hey....?

  14. #14
    Master Apprentice phantomotap's Avatar
    Join Date
    Jan 2008
    Posts
    5,108
    everyone is having a nice old laugh watching me solve this riddle hey....?
    I don't see anyone laughing.

    The post by jimblumberg more or less "spoon feeds" the answer.

    The posts by anduril462 are explaining why the answer is the answer so that you might learn more than just the literal answer.

    The truth is, I really don't see anything more than can be added without just restating with my words what they've said.

    You've actually rephrased what they said well enough that I think you could not possibly have managed it without understanding the issues in question.

    *shrug*

    You apparently understand the issues well enough that your descriptions of both "integer overflow" and "buffer overflow" are pretty good.

    So, again, what aren't you getting about the posts here?

    Soma

  15. #15
    Registered User
    Join Date
    May 2012
    Posts
    12
    Ok.....is this a buffer overflow.....as simple as that.

    What happens is this....... i will always be the same value as the buf_leng, and [I]buf_len will be the same size as buf[]
    which means.....when the function bufruns it accesses the last value in the array......and then adds one more char because '/0' still indicates a byte.
    which is one more than the array...can hold.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Find vulnerabilities in this C code?
    By Avison in forum C Programming
    Replies: 5
    Last Post: 03-12-2012, 08:42 AM
  2. system() function security vulnerabilities?
    By anonytmouse in forum Tech Board
    Replies: 8
    Last Post: 11-11-2004, 10:10 AM
  3. Having trouble translating psudeo-code to real-code.
    By Lithorien in forum C++ Programming
    Replies: 13
    Last Post: 10-05-2004, 07:51 PM
  4. Texts on Format String Vulnerabilities?
    By Tal0n in forum A Brief History of Cprogramming.com
    Replies: 0
    Last Post: 01-09-2004, 09:18 AM