7

My class is without a textbook, or any structured learning material for that matter. I am doing all my learning through googling and while I am picking it up as I am doing it, it is slow going to be honest.

Class assignment:

There is a program (psuedocode somewhat) in the file prog-bad.c. This program is not written to be compiled or run, but to be read.

Find what the vulnerabilities are, and what these vulnerabilities may cause and write a short explanation of what you found.

#include <stdio.h>
int main() {
int studentId;
char studentName[100], buffer[100], percentage[10];

char* firstName = malloc(sizeof(char)*50);
char* lastName = malloc(sizeof(char)*50);
char* graduateLevel = NULL;

printf("\n**********Welcome to Student Registration Service**********\n");
printf("\nPlease enter your 10 digit Student ID: ");
scanf("%d", &studentId);

printf("\nPlease enter your first name: ");
gets(firstName);

printf("\nPlease enter your last name: ");
gets(lastName);

{
    char level[2];
    printf("\nPlease enter your graduate level (UG/PG): ");
    gets(level);
    graduateLevel = &level;
}

strcpy(studentName, firstName);
strcat(studentName, lastName);

printf("\nPlease enter your percentage in Highschool (like 85.50%): ");
gets(percentage);
// Student record saving in file
updateStudentRecord();

printf("\nStudent record saved with the following details: \n");
printf("\nStudent ID: %d", studentId);
printf("\nStudent Name: %s", studentName);
printf("\nStudent percentage in Highschool: ");

printf(percentage);

free(firstName);
free(firstName);

return(0);
}

For my answer I am talking about the use of all the scanf, gets, and printf functions opening up the program to buffer overflow attacks. But I am a little conflicted cause I see that the program makes use of malloc, which I thought was a way to securing up the stack by moving everything to the nonexacutable heap.

So does this mean that the program is not as unsecured as I thought and it is a trick question of sorts?

2

Malloc is used for -

char* firstName = malloc(sizeof(char)*50);
char* lastName = malloc(sizeof(char)*50);

You are right in that these are on the heap. But what then happens to this data? And how do these functions work?

What else is read from the user? And how?


Reply to your comment below

So as I see it in my limited scope and understanding, the vaulerabilities are: all the printf, the scanf for studentid

studentid is an int. So I believe the operations on it are safe.

, gets for first and last name

But because malloc was used for the first and last name, does that kinda safegaurd all instances of the gets for them.

The data immediately after the gets will be on the heap yes. But does this make it safe? What happens if the user puts in more than 50 characters for either field? I'd suggest reading up on Heap Overflow. Even if corrupting heap data wasn't an issue look at what the code then does with that data. What will the strcpy and strcat do?

gets for level / gets for percentage

Yes. Not only can these both be caused to overflow but what happens after the "graduateLevel = &level;" line?


Reply to second comment -

confused as about you last point and what happens after "graduateLevel=&level;"?

"char level[2];" is declared within the curly brace directly above. This means it is only protected within that blocks scope. That scope ends on the line after "graduateLevel = &level;". So graduateLevel points to the address where level used to be. But the compiler no longer has any obligation to make sure level is not overwritten. Any operations using level after this point are undefined behavior.

Now this issue is pretty minor. There is a fairly high chance the compiler won't reuse it. Compared to level being read in from a gets call above its negligible. But code like this frequently leads to odd and hard to track down bugs - some of which can be exploited.

I am bit confused cause I have read a few places that strcpy and strcat are issues, but I have also found that they are not?

First of all you need to read and understand the definitions for strcpy and strcat. Lets take strcpy -

Copies the null-terminated byte string pointed to by src, including the null terminator, to the character array whose first element is pointed to by dest.

So, it copies characters out of the source array into the destination array until it hits a null terminator ('\0' - usually byte value 0). Notice the warnings -

The behavior is undefined if the dest array is not large enough. The behavior is undefined if the strings overlap. The behavior is undefined if either dest is not a pointer to a character array or src is not a pointer to a null-terminated byte string.

So - for the line

strcpy(studentName, firstName);

do these apply? Dest is studentName - a 100 character array. Source is firstName - a pointer to a 50 character array. But is firstname null-terminated? Well its value came from -

gets(firstName);

gets is defined as -

Reads stdin into the character array pointed to by str until a newline character is found or end-of-file occurs. A null character is written immediately after the last character read into the array. The newline character is discarded but not stored in the buffer.

With the warning -

The gets() function does not perform bounds checking, therefore this function is extremely vulnerable to buffer-overflow attacks. It cannot be used safely (unless the program runs in an environment which restricts what can appear on stdin). For this reason, the function has been deprecated in the third corrigendum to the C99 standard and removed altogether in the C11 standard. fgets() and gets_s() are the recommended replacements.

Never use gets().

So. If I were to enter "Dylan" in firstName would contain 'Dylan\0'. What happens if I entered in 200 bytes?

The standard says its undefined but for most real systems its easy enough to guess. Its just going to keep writing into whatever memory is after firstName. What happens then? Well - if it leaves the memory space the process can legally write to the OS is probably going to kill it. But if it doesn't then it will keep running oblivious until either something goes wrong enough to cause it to crash (whatever was supposed to be stored back there isn't any more) or the program terminates. Now as you pointed out the damage is limited - this data is stored on the heap.

So. Lets assume everything continues with no issues until the strcpy. What happens there? strcpy does not know firstName is only supposed to be 50 characters. Or that studentName is only supposed to be 100 characters. Its just going to keep copying until it hits a null terminator.

lastName and strcat have exactly the same issue.

  • So as I see it in my limited scope and understanding, the vulnerabilities are: all the printf, the scanf for studentid, gets for first and last name, gets for level, the strcopy and strcat for the names, and the gets for percentage. But because malloc was used for the first and last name, does that kinda safeguard all instances of the gets for them. I guess I am still confused as to how these are used so that they cause issues or don't cause issues. – Dylan Slater Oct 31 '17 at 15:43
  • Updated my reply to answer this comment. – Hector Oct 31 '17 at 15:56
  • So I have done some more reading up to try and understand the paths you lead me too. But I am still a bit confused as about you last point and what happens after "graduateLevel=&level;"? And I am bit confused cause I have read a few places that strcpy and strcat are issues, but I have also found that they are not? – Dylan Slater Nov 2 '17 at 22:08
  • Extended my answer again. – Hector Nov 3 '17 at 9:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.