I have this code in which I have to find security vulnerabilities.

win() {
    printf("Congratulations!");
}
setter(unsigned int i, int v, int * a) {
    a[i] = v;
}

main() {
    int i, v;
    int a[50];
    puts("i");
    scanf("%d", i);
    puts("v");
    scanf("%d", v);
    setter(i, v, a)
}

Reading on the Internet, I've found that when a conversion from an int to an unsigned int takes place, if the int is a negative number, it will be converted into a very high unsigned int. So, probably the vulnerabilty in this code is due to this conversion which takes place when setter is called in the main function. So, an attacker could give the following input: i=-1, v=40 and he will access to a part of the memory which isn't reserved to the buffer a and so, in the setter function, the attacker will be able to overwrite important values. For example, the attacker could overwrite the return address with the address of the win function in order to execute win when setter returns. Am I right?

CodePudding user response：

Summary of some comments and some additions

Firstly, the code is wrong. The arguments to scanf needs to be pointers:

scanf("%d", &i); // Note the &

if the int is a negative number, it will be converted into a very high unsigned int.

Yes, this is technically true, and it's also true that this may lead to accessing the array out of bounds. However, even if you change to unsigned, there is nothing preventing the attacker from simply typing any number, including whatever the number (unsigned) -1 will be on the particular system.

The correct way of solving this security issue is to check that the inputted number are in the range [0,49]. If you use an unsigned number, you may skip checking if it's below zero for obvious reasons. But you still need to do a range check.