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

如何区分source的原始对象和传播中被污染的对象 #63

Closed
firmianay opened this issue Nov 17, 2023 · 12 comments
Closed

如何区分source的原始对象和传播中被污染的对象 #63

firmianay opened this issue Nov 17, 2023 · 12 comments

Comments

@firmianay
Copy link
Contributor

https://codeql.github.com/codeql-query-help/java/java-android-intent-uri-permission-manipulation/

下面分别是有漏洞和没漏洞的例子:

    // BAD: the user-provided Intent is returned as-is
    public void dangerous() {
        Intent intent = getIntent();
        intent.putExtra("result", "resultData");
        setResult(intent);
    }

    // GOOD: a new Intent is created and returned
    public void safe() {
        Intent intent = new Intent();
        intent.putExtra("result", "resultData");
        setResult(intent);
    }

那如果是这种情况,intent1虽然是新建的,但是被intent2污染了,如果不能区分这两者,那就会产生误报:

    public void safe() {
        Intent intent1 = new Intent();
        Intent intent2 = getIntent();
        String string2 = intent2.getDataString();
        intent1.putExtra("result", string2);
        setResult(intent1);
    }
@nkbai
Copy link
Collaborator

nkbai commented Nov 18, 2023

通过 throughapi 辅助 判断,是否可行?

@firmianay
Copy link
Contributor Author

感觉不行,不管through了什么api,最后都会回到如何区分这两个对象的问题

@nkbai
Copy link
Collaborator

nkbai commented Nov 20, 2023

感觉不行,不管through了什么api,最后都会回到如何区分这两个对象的问题

比如Intent.getParcelable?,主要是从本质上区分,从Intent中直接提取一个Intent,如果只是从Intent中提取string,integer之类的,这种污点传播,认为是没有危害的。

@firmianay
Copy link
Contributor Author

比如Intent.getParcelable?,主要是从本质上区分,从Intent中直接提取一个Intent,如果只是从Intent中提取string,integer之类的,这种污点传播,认为是没有危害的。

对,所以就产生了这个误报,因为只有setResult(intent2)才是漏洞,还是说intent1.putExtra("result", string2);并不会导致intent1被污染,因此不会有这条链,引擎里对这种污点传播有特殊处理吗?

@nkbai
Copy link
Collaborator

nkbai commented Nov 20, 2023

刚刚的throughpapi就是设计用来做这个事情的。 具体是什么api,需要你自己根据你的业务代码进行总结。

@nkbai
Copy link
Collaborator

nkbai commented Nov 20, 2023

比如Intent.getParcelable?,主要是从本质上区分,从Intent中直接提取一个Intent,如果只是从Intent中提取string,integer之类的,这种污点传播,认为是没有危害的。

对,所以就产生了这个误报,因为只有setResult(intent2)才是漏洞,还是说intent1.putExtra("result", string2);并不会导致intent1被污染,因此不会有这条链,引擎里对这种污点传播有特殊处理吗?

这个本质是如何操控�pathfinder,你看看有什么其他的sanitizer方式,可以自定义加进去的。

@firmianay
Copy link
Contributor Author

firmianay commented Nov 20, 2023

限定在getParcelable并不能解决这种类型的误报:

    public void safe() {
        Intent intent1 = new Intent();
        Intent intent2 = getIntent().getExtras().getParcelable(key)
        String string2 = intent2.getDataString();
        intent1.putExtra("result", string2);

        setResult(intent2);  // 漏洞
        setResult(intent1);  // 误报
    }

这种通用漏洞似乎应该假设跟业务代码没关系,当然如果是case by case,熟悉业务逻辑的话,确实可以针对性做一些限制。你们有没有那种仅针对某个App的规则,增加个packageName字段?

@nkbai
Copy link
Collaborator

nkbai commented Nov 20, 2023

确实不能,针对性的限制,可以举例说说么?只要能在sanitizer中表达的,都是可以的。
工具本来就是在误报和漏报之间寻求平衡。

@firmianay
Copy link
Contributor Author

firmianay commented Dec 18, 2023

我分析了很多样本,发现最常见的代码模式是这样的:

val str = getIntent().getStringExtra("str")
val newIntent = Intent()
newIntent.putExtra("str", str)
setResult(1, newIntent)

所以我尝试去检查put*这种函数,用NotTaint去排除新建的Intent,即污点没有污染@this,但是污染了p*

    sanitizer: {
      newIntentPut: {
        "<android.content.Intent: android.content.Intent put*(*)>": {
          TaintCheck: ["p*"],
          NotTaint: ["@this"],
        },
      },

但是好像不太行,appshark认为@this也是污染的,就感觉NotTaint是指执行完put*这条程序之后的情况?因为执行前@this是干净的,只有p*被污染。

你们有规则用到NotTaint: ["@this"]吗,感觉执行后的情况是没有意义的,因为传播规则里有param->@this,肯定会被污染,这块能改成执行前检查吗?

@nkbai
Copy link
Collaborator

nkbai commented Dec 18, 2023

appshark进行的是流不敏感 分析,所以可以直接忽略指令间的顺序

@nkbai
Copy link
Collaborator

nkbai commented Dec 18, 2023

@firmianay
Copy link
Contributor Author

appshark进行的是流不敏感 分析,所以可以直接忽略指令间的顺序

那可能没戏了...改VariableFlowRule的话,删了put*param->@this其他分析估计也进行不下去了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants