Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Restructuring] Side effects #19

Open
ebehner opened this issue Jan 12, 2022 · 2 comments · May be fixed by #93
Open

[Restructuring] Side effects #19

ebehner opened this issue Jan 12, 2022 · 2 comments · May be fixed by #93
Assignees
Labels
bug Something isn't working priority-medium Medium priority issue

Comments

@ebehner
Copy link
Collaborator

ebehner commented Jan 12, 2022

What happened?

When we restructure a CFG into an AST, we transform each condition into a symbol. Now, each node of a region gets its reaching condition assigned, thus a reaching condition can occur multiple times.

Consider the following example:
rc-Problem-126

The putout is incorrect because the value of a changes and afterward, we have a check with the old value.

An correct output would, for example be:

cond_b1 = (a == 0)
if(cond_b1){
    a = 1;
}
if(! cond_b1 && b < 10){
    b = b*a;
}else{
    b = b-a;
}
return b;

How to reproduce?

A binary where this problem also occurs is test_switch.zip test18.
The C-code is:

int test18()
{
    int week;
    //Non sequential case constants
    
    /* Input week number from user */
    printf("Enter week number(1-7): ");
    scanf("%d", &week);
    
    switch(week)
    {
        case 1: 
            printf("Monday");
            week +=500 ;
        case 12: 
            printf("Tuesday");
            break;
        case 500: 
            printf("Friday");
            // break;
        default: 
            printf("Invalid input! Please enter week number between 1-7.");
    }
    printf("the number is %d", week);
    return 0;

}

and the output is:

int test18() {
    int var_0;
    int * var_1;
    printf("Enter week number(1-7): ");
    var_1 = &var_0;
    __isoc99_scanf(0x804c025, var_1);
    switch(var_0) {
    case 1:
        printf("Monday");
        var_0 += 0x1f4; // <-------- var_0 is changed here, but the old value should be used for the comparison in the if-statement
        break;
    case 0x1f4:
        printf("Friday");
        break;
    }
    if ((var_0 != 1) && (var_0 != 12)) { // <----------- If var_0 was 1 when reaching the switch, it is now != 1
        printf("Invalid input! Please enter week number between 1-7.");
    }
    else {
        printf("Tuesday");
    }
    printf("the number is %d", var_0);
    return 0;
}

When entering the number 1, the function prints "Monday", "Tuesday", and "the number is 501".
But the decompiled function, with input 1 prints "Monday", "Invalid input! Please enter week number between 1-7.", and "the number is 501".

Affected Binary Ninja Version(s)

Version 2.4.2846

@ebehner ebehner added bug Something isn't working priority-high High priority issue labels Jan 12, 2022
@ebehner ebehner changed the title [Restructuring] [Restructuring] Wrong Value in Condition Jan 12, 2022
@ebehner ebehner added priority-medium Medium priority issue and removed priority-high High priority issue labels Feb 21, 2022
@ebehner
Copy link
Collaborator Author

ebehner commented Jul 4, 2022

/cib

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2022

Branch issue-19-_Restructuring_Wrong_Value_in_Condition created!

github-actions bot pushed a commit that referenced this issue Jul 4, 2022
@github-actions github-actions bot linked a pull request Jul 4, 2022 that will close this issue
@ebehner ebehner self-assigned this Jul 14, 2022
ebehner added a commit that referenced this issue Aug 12, 2022
@rihi rihi changed the title [Restructuring] Wrong Value in Condition [Restructuring] Side effects Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-medium Medium priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant