sensepost / objection

📱 objection - runtime mobile exploration
GNU General Public License v3.0
7.46k stars 854 forks source link

[bug] --dump-args and --dump-return not working correctly. #415

Closed ido77778 closed 3 years ago

ido77778 commented 4 years ago

Describe the bug When adding the parameters --dump-arguments and --dump-return when hooking to a method, the argument and returns values are given as [Object object] whenever the object isn't a primitive, rather than their specific values. I was able to trace down the offending code to agent/src/android/hooking.ts where

if (dargs && calleeArgTypes.length > 0) {
  const argValues: string[] = [];
  for (const h of arguments) {
    argValues.push((h || "(none)").toString());
  }

and

if (dret) {
  const retValStr: string = (retVal || "(none)").toString();
  send(c.blackBright(`[${job.identifier}] `) + `Return Value: ${c.red(retValStr)}`);
}

Were changed to use JSON.Stringify() in commit ac40784b7a8296e714def56a62df3473bbe41487. Oddly enough, the commit was made to fix #334, which is the exact same problem. When I revert that commit, the problem it was supposed to solve disappears (!). Acquaintances of mine also confirm this to have been an issue.

To Reproduce

  1. Run objection.
  2. Hook to any non-void method: android hooking watch class_method [method URI] --dump-args --dump-return.
  3. Watch as objects are passed around.

Expected behavior Giving the parameters --dump-arguments and --dump-return should output the arguments and return values, not just their types.

Evidence / Logs / Screenshots Defective behavior after ac40784b7a8296e714def56a62df3473bbe41487: Screenshot_20201008_145424

Correct behavior after reversion: Screenshot_20201008_145045

Environment (please complete the following information):

Application I used the following simple code (on a MainActivity) to debug this issue:

public class MainActivity extends AppCompatActivity
{
    private static final String TAG = "MainActivity";

    @Override
    protected void onCreate(Bundle savedInstanceState)
    {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
    }

    public void btn1Click (View view)
    {
        JSONObject obj = new JSONObject();
        try
        {
            obj.put("test", "test1");
            obj.put("test2", "test3");
        }
        catch (JSONException e)
        {
            e.printStackTrace();
        }

        JSONObject objReturn = reproducerJSON(obj);
        Log.w(TAG, "Run Successful!");

        ComplexNumber complexNumber = new ComplexNumber(1, 4);
        ComplexNumber complexReturn = reproducerComplex(complexNumber);
    }

    protected JSONObject reproducerJSON(JSONObject exampleObject)
    {
        return exampleObject;
    }
    protected ComplexNumber reproducerComplex(ComplexNumber exampleComplex) { return exampleComplex; }
}

I also made the following simple ComplexNumber class to test whether it was only an issue with JSON files, and it confirmed the problem occures with any object, and is still fixed after the change: package com.example.reproducer;

public class ComplexNumber
{
    private final int realComponent;
    private final int imaginaryComponent;

    public ComplexNumber(int real, int imaginary)
    {
        this.realComponent = real;
        this.imaginaryComponent = imaginary;
    }

    @Override
    public String toString() { return realComponent + " i" + imaginaryComponent; }
}
nirdev commented 4 years ago

Any updates on merging this to the master?this issue affects me to... @leonjza

leonjza commented 4 years ago

Sorry, I am spread pretty thin at the moment. I'll get to it as soon as possible.

OrDuan commented 3 years ago

That explains the issue we had when we upgraded Objection, it broke our internal tool, that fix is highly needed :+1:

nirdev commented 3 years ago

Any updates @leonjza?

leonjza commented 3 years ago

Not yet, no.

leonjza commented 3 years ago

Sorry this took so long. I just landed #414 which will be available in the next release.