Skip to content
New issue

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

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

Already on GitHub? # to your account

handle field / method name collision #21

Merged
merged 4 commits into from
Mar 2, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/class-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -669,9 +669,13 @@ function ClassFactory (vm) {
try {
const fieldName = invokeObjectMethodNoArgs(env.handle, field, fieldGetName);
try {
const fieldjsName = env.stringFromJni(fieldName);
var fieldjsName = env.stringFromJni(fieldName);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use let here to allow re-assigning – var is a legacy keyword from ES5.

const fieldHandle = env.newGlobalRef(field);
fieldHandles.push(fieldHandle);
// If we have a collided method, suffix the fieldName
if(jsMethods.hasOwnProperty(fieldjsName)){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Comment can be removed as it's already clear what the code is doing.
  • Missing space after if.
  • Theoretically there might be a method prefixed with _ having the same name, so using while here would be better.

fieldjsName = "_" + fieldjsName;
}
jsFields[fieldjsName] = fieldHandle;
} finally {
env.deleteLocalRef(fieldName);
Expand Down
70 changes: 70 additions & 0 deletions test/re/frida/MethodTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,62 @@ public void genericsCanBeUsed() {
assertEquals("Badger", script.getNextMessage());
}


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra blank line here.

@Test
public void fieldsThatCollideWithMethodsGetSuffixed() {
loadScript("var Collider = Java.use('re.frida.Collider');" +
"var collider = Collider.$new();" +
"send(collider._Particle);");
assertEquals("1", script.getNextMessage());
}

@Test
public void methodsThatCollideWithFieldsKeepName() {
loadScript("var Collider = Java.use('re.frida.Collider');" +
"var collider = Collider.$new();" +
"send(collider.Particle());");
assertEquals("3", script.getNextMessage());
}

@Test
public void fieldsThatCollideWithMethodsGetSuffixed2() {
loadScript("var Collider = Java.use('re.frida.Collider');" +
"var collider = Collider.$new();" +
"send(collider._Particle2);");
assertEquals("2", script.getNextMessage());
}

@Test
public void methodsThatCollideWithFieldsKeepName2() {
loadScript("var Collider = Java.use('re.frida.Collider');" +
"var collider = Collider.$new();" +
"send(collider.Particle2());");
assertEquals("4", script.getNextMessage());
}

@Test
public void collidedMethodsFieldsCanStillBeInstrumented() {
loadScript("var Collider = Java.use('re.frida.Collider');" +
"Collider._Particle.implementation = function () {" +
"return 11;" +
"};" +
"Collider._Particle2.implementation = function () {" +
"return 22;" +
"};" +
"Collider.Particle.implementation = function () {" +
"return 33;" +
"};" +
"Collider.Particle2.implementation = function () {" +
"return 44;" +
"};");

Collider collider = new Collider();
assertEquals(11, Collider.Particle);
assertEquals(22, collider.Particle2);
assertEquals(33, collider.Particle());
assertEquals(44, Collider.Particle2());
}

// @Test
public void interfaceCanBeImplemented() {
loadScript("var X509TrustManager = Java.use('javax.net.ssl.X509TrustManager');" +
Expand Down Expand Up @@ -118,3 +174,17 @@ public int returnZero() {
return 0;
}
}

class Collider {
static int Particle = 1;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespace.

int Particle2 = 2;

int Particle() {
return 3;
}

static int Particle2(){
return 4;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Field- and method-names should follow the coding style, i.e. camelCase, not CamelCase.

}