HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Corrupting Java arithmetic through reflection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
reflectioncorruptingjavathrougharithmetic

Problem

Inspired by a PPCG answer, I wrote this code, which shuffles the autoboxing caches of Number subclasses.

This leads to, among other things, outputting 2 + 2 = -72 after invocation of EvilStuff.doEvilStuff(Class,String,int). (The result differs for different invocations, due to reshuffling).

I use a standard Fisher-Yates shuffle. I use reflection to access the autoboxing caches by the designated cacheID and cacheFieldNumber parameters, which are cache and 0 for java.lang.Integer. java.lang.Integer is the only class which implements an effective autoboxing cache other than java.math.BigDecimal, and that is not used much for normal arithmetic. Also, the Integer stuff is better documented.

I figured I would do this since no one would otherwise think of posting evil code on Code Review, and to serve to warn myself of the dangers of using reflection in the future.

Welcome to the dark side! How would anyone else do this?

```
/**
* Messes up arithmetic for some cases using reflection to shuffle cache stuffs. Enjoy!
*/
public class EvilStuff{
/**
* Does a Fisher-Yates shuffling of the input.
* @param stuff The array to shuffle
* @return Nothing, array is sorted in-place
*/
public static void shuffle(Object[] stuff){
int length = stuff.length;
for (int i = 0; i victim,String cacheID, int cacheFieldNumber){
Class cache = victim.getDeclaredClasses()[cacheFieldNumber];
java.lang.reflect.Field c;
try{
c = cache.getDeclaredField(cacheID);
}catch(NoSuchFieldException noCache){
throw new IllegalArgumentException(String.format("Can't mess with something (%s) without a cache (%s) at %d!"
,victim+"",cacheID,cacheFieldNumber), noCache);
}
c.setAccessible(true);
Object cached;
try{
cached=c.get(cache);
}catch(IllegalAccessException noAccessPermit){
throw new IllegalArgumentExcepti

Solution

Regarding shuffle:

  • Declare length in the for loop.



  • Use Random.random(int) instead of (int)(Math.random() * int).



  • Rename random to swapIndex or something more descriptive.



  • With these changes the comments are unnecessary.



  • This method needn't be public.



  • Also, I made things final although that's a matter of style.



My version:

private static void shuffle(final Object[] stuff){
    final Random r = new Random();
    for (int i = 0, length = stuff.length; i < length; i++) {
        final int swapIndex = i + r.nextInt(length - i);
        final Object randomElement = stuff[swapIndex];
        stuff[swapIndex] = stuff[i];
        stuff[i] = randomElement;
    }
}


Regarding doEvilStuff:

The proliferation of try/catches really clutters the code, especially because you rethrow an IllegalArgumentException each time. Your code would be more readable if you wrapped the whole method in a try/catch and used a multi-catch. Actually I would be controversial and advocate catching Exception because no matter what goes wrong, you want to wrap the exception and provide diagnostic information and it's impractical (and impossible without depending on implementation details) to list every exception that could be thrown. Note that catching Exception is bad in most circumstances.

However, with this approach of having just one catch block your error messages can't be quite as descriptive. That's okay as long as the thrown exception includes the parameters and the original exception - any more is IMO excessive.

Another issue with your code is it puts the burden of specifying the index of the nested class on the caller. As all nested classes of Number type X are named XCache, we may use this pattern in doEvilStuff to automatically find the correct nested class.

My version:

public static void doEvilStuff(Cfinal lass victim,String cacheID) {
    try {
        final Class cache = Class.forName(victim.getName() + "$" + victim.getSimpleName() + "Cache");
        final java.lang.reflect.Field c = cache.getDeclaredField(cacheID);
        c.setAccessible(true);
        shuffle((Number[])c.get(cache));
    } catch (final Exception e) {
        throw new IllegalArgumentException("can't corrupt class " + victim, e);
    }
}

Code Snippets

private static void shuffle(final Object[] stuff){
    final Random r = new Random();
    for (int i = 0, length = stuff.length; i < length; i++) {
        final int swapIndex = i + r.nextInt(length - i);
        final Object randomElement = stuff[swapIndex];
        stuff[swapIndex] = stuff[i];
        stuff[i] = randomElement;
    }
}
public static void doEvilStuff(Cfinal lass<? extends Number> victim,String cacheID) {
    try {
        final Class cache = Class.forName(victim.getName() + "$" + victim.getSimpleName() + "Cache");
        final java.lang.reflect.Field c = cache.getDeclaredField(cacheID);
        c.setAccessible(true);
        shuffle((Number[])c.get(cache));
    } catch (final Exception e) {
        throw new IllegalArgumentException("can't corrupt class " + victim, e);
    }
}

Context

StackExchange Code Review Q#123247, answer score: 4

Revisions (0)

No revisions yet.