C homework - trojan found when compiling the code

Question

I was coding some homework (101 level). When I tried to compile the code, I got some virus alert from bit defender:

#include <stdio.h>

int main ( void ) {
int numbers [10];
int i, temp;

for (i = 1; i <= 10; ++i)
    numbers[i] = 0;

printf("Enter up to 10 integers. input '-1' to finish \n");

for (i = 0; i < 10; i++) {
    scanf("%d", &temp);
    if (temp == -1) {
        break;
    } else {
        numbers [i] = temp - 1;
    }
}

for (i = 1; i <= 10; ++i)
    printf("the numbers are: %d\n", numbers[i]);

return 0;
}

virus alert print screen

I believe the problem is with this piece of code:

    for (i = 1; i <= 10; ++i)
        numbers[i] = 0;

Why the trojan virus alert? what did I do?

don't think that has anything to do with your homework, it seems like you have some viruses running around. — iabdalkader, Nov 17 '12 at 21:33
wow, if this is really because of your code, this is an extremely paranoid virus scanner.. You're accessing memory which doesn't belong to your application. Array indices start at 0. — stefan, Nov 17 '12 at 21:34
You *are* overrunning the array bounds, but I would be very surprised if your virus scanner could pick that up. — Kerrek SB, Nov 17 '12 at 21:34
@KerrekSB I'm still interested why the scanner picks that up. More specifically, why does it think buffer overflow = Meur.GZ — John Dvorak, Nov 17 '12 at 21:38
@JanDvorak: it's highly unlikely that anyone could pick up the array overrun from just the compiled code, and *especially* not statically... — Kerrek SB, Nov 17 '12 at 21:40
@KerrekSB It may not not be the array overrun, but a _stack_ overflow is not that hard to notice by simulation. In this case, an array overrun triggers a stack overflow (and a question to StackOverflow). — John Dvorak, Nov 17 '12 at 21:41
@JanDvorak: are you sure? This is a static array after all... and do virus scanners actually *run* the code in question? — Kerrek SB, Nov 17 '12 at 21:48
@JanDvorak no commercial/free antivirus is that complicated, mostly they just match virus signatures from databases, I did read a paper once though about some anti obfuscation techniques and static analysis of viruses. — iabdalkader, Nov 17 '12 at 21:49
@mux I sincerely hope at least _some_ antiviruses actually run the code they are supposed to check. — John Dvorak, Nov 17 '12 at 21:50
@JanDvorak don't think they should run it at all ! some work was done with neural networks too, that's all I know of. — iabdalkader, Nov 17 '12 at 21:52
@mux why do you think they _shouldn't_ run it (in a sandbox, ofc)? — John Dvorak, Nov 17 '12 at 21:53
@JanDvorak even if you do, how would you know that it's a virus ? could be any program trying to read/write or in this case, just buggy. — iabdalkader, Nov 17 '12 at 21:57
@mux It's about weighing the pros/cons. I bet a rare false positive is preferable to a yet rarer false negative. You can turn off the filter if you really hate false positives anyways. — John Dvorak, Nov 17 '12 at 22:00
@mux yould you let an armed person into a school? Perhaps he won't be trying to kill the children. — John Dvorak, Nov 17 '12 at 22:05
@JanDvorak you have a point, but a high false positive rate is useless, I've never seen any antivirus that runs viruses because it's too risky, anyway. — iabdalkader, Nov 17 '12 at 22:09
@mux How is running something within a sandbox risky? I agree a high false positive rate is useless, but then again, not many programs consistently overflow. I even guess not many programs ever overflow (and those that do deserve a virus alert). — John Dvorak, Nov 17 '12 at 22:13
@mux: you have no idea what you are talking about. Read on Generic Decryption Engines, some of them are x86 emulators. — ninjalj, Nov 17 '12 at 22:29
@JanDvorak: note it's _Heur_ (for heuristic), not _Meur_. So, it detects a generic trojan via heuristic analysis. — ninjalj, Nov 18 '12 at 23:32

Alberto Bonsanto · Accepted Answer · 2012-11-18T13:53:33.130

Don't pay attention some antivirus programs recognize the compiled items as virus, it does the same avast with visual studio, just add exception to your antivirus list. But your code has some problems indeed.

for (i = 1; i <= 10; ++i) is incorrect, because the arrays in C start on 0, and second to initialize variables you don't need to do for loops you can assign them values like any other variable.
numbers [i] = temp - 1 The way you store the values in the array is not so good, because you are altering the inputed values when you do -1.

a

/*For the array initialization.*/
int numbers[10] = { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 };

/*For inputing the values.*/

for ( i = 0; i < 10; i++ ){
    scanf( "%d", &temp );
    if( temp == -1 ){
        numbers[ i ] = -1;
        break;
    else{
        numbers[ i ] = temp;
    }
}

/*For the printing. */

for( i = 0; i < 10 ; i++ ){
    if( numbers[ i ] == -1 ){
        break;
    }
    printf( "numbers[%d] is %d", i, numbers[ i ] );
}

so, if i enter 10 valid numbers i will face an array with 10 valid numbers. how often will the `!= -1` check in the printing part actually catch the end of the array? — akira, Nov 17 '12 at 21:54
`if( numbers[ i ] = -1 )` will always evaluate as true. Use `==` instead. — John Dvorak, Nov 17 '12 at 22:15

score 4 · Answer 2 · answered Nov 17 '12 at 21:34

4

you trigger a buffer-overflow. your array 'numbers' is 10 items big, you access the 11th item.

answered Nov 17 '12 at 21:34

akira

6,050
29
37

Omkant · Answer 3 · 2012-11-17T21:45:38.983

4

Use i=0 instead of i =1 because in C array indexes start at 0

size of array is 10 so the last index is 9 So you are accessing the array index which is out of bound in numbers[10], so it's undefined behaviour

Array would be like this :

numbers[0] ,numbers[1], . . . numbers[9]

modify code to this :

for(i=0;i<10;i++)
  printf("%d\t",numbers[i]);

edited Nov 17 '12 at 21:45

answered Nov 17 '12 at 21:34

Omkant

9,018
8
39
59

as long as he stays within the boundaries of the array: no problem where he starts or ends. – akira Nov 17 '12 at 21:35

C homework - trojan found when compiling the code

3 Answers3