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

Taint summary requires "source_lineno" and "sink_lineno" #3

Open
mafsbaptista opened this issue Jan 24, 2024 · 4 comments
Open

Taint summary requires "source_lineno" and "sink_lineno" #3

mafsbaptista opened this issue Jan 24, 2024 · 4 comments

Comments

@mafsbaptista
Copy link

I tried to remove the "source_lineno" and "sink_lineno" from the taint summaries but when trying to generate the symbolic tests, the following error appears:
Instrumentor2: internal error, uncaught exception: Yojson__Basic.Util.Type_error("Expected int, got null", 870828711).
It's far from critical, I re-added the lines and it's working :)

@filipeom
Copy link
Member

filipeom commented Jan 30, 2024

Reopening as I think vuln_type withing the nested return assoc should be optional:

  {
    "vuln_type" : "code-injection",
    "source" : "f1",
    "sink" : "anon",
    "tainted_params" : [ "a" ],
    "params_types" : { "a" : "string" },
    "return" : {
      "vuln_type" : "code-injection", <- this
      "source" : "",
      "sink" : "eval",
      "tainted_params" : [ "b" ],
      "params_types" : { "b" : "number" }
    }

We can use this issue to determine the rest of json associations that can be optional:

  • source_lineno
  • sink_lineno
  • nested vuln_type
  • nested sources ?

Current summary type:

type vuln_conf =
  { ty : vuln_type
  ; source : string
  ; source_lineno : int option (* optional *)
  ; sink : string
  ; sink_lineno : int option (* optional *)
  ; tainted_params : string list
  ; params : (string * param_type) list
  ; return : vuln_conf option (* optional *)
  }

@mafsbaptista
Copy link
Author

I agree, I think all of those can be optional. In the nested return we don't need to know the source right? We already know that we will call the return value, so the function name isn't required, right? I'm saying this because sometimes is difficult to know this due to the normalization.

@filipeom
Copy link
Member

In the nested return we don't need to know the source right? We already know that we will call the return value, so the function name isn't required, right?

Correct. However, we probably need another assoc type, perhaps named return_obj, instead of return, for situations where the exported function returns an object. In such cases, we could use source to determine which property of the exported object to call. I'm considering examples like this:

module.exports = function() {
	function Obj(a) { this.a = a }
	Obj.prototype.f = function(b) {
		if (b > 0) {
			eval(this.a);
		}
	}
	return Obj;
}

Where we want to call Obj.f.

The summary would be:

  {
    "vuln_type" : "code-injection",
    "source" : "module.exports",
    "tainted_params" : [ ],
    "params_types" : { },
    "return_obj" : {
      "source" : "f",
      ...
    }

@mafsbaptista
Copy link
Author

True. I haven't done those queries but I think I'll be able to do the summaries like that.

@filipeom filipeom transferred this issue from formalsec/instrumentation2 Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants