Skip to content

Commit

Permalink
Allow address 0 when bytesCount is 0 for copyMemory and setMemory
Browse files Browse the repository at this point in the history 
Resolves #3
  • Loading branch information
@Marcono1234
Marcono1234 committed May 1, 2024
1 parent 80cb8a6 commit 220c247
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 8 deletions.
17 changes: 11 additions & 6 deletions agent-impl/src/main/java/marcono1234/unsafe_sanitizer/agent_impl/MemoryTracker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ public boolean onAccess(long address, long bytesCount, boolean isRead) {
}

public boolean onCopy(long srcAddress, long destAddress, long bytesCount) {
if (bytesCount == 0) {
// Allow address to be right behind section ('exclusive end address') since copy size is 0
boolean isZeroSized = true;

// Allow address 0; this assumes that user code might first call `Unsafe#allocateMemory` and then uses
// the returned address as argument to `copyMemory`;
// if the bytes count is 0, then `allocateMemory` will return 0 as address
return (srcAddress == 0 || verifyValidAddress(srcAddress, isZeroSized))
&& (destAddress == 0 || verifyValidAddress(destAddress, isZeroSized));
}

if (srcAddress <= 0) {
return reportError("Invalid srcAddress: " + srcAddress);
} else if (destAddress <= 0) {
Expand All @@ -238,12 +249,6 @@ public boolean onCopy(long srcAddress, long destAddress, long bytesCount) {
return false;
}

if (bytesCount == 0) {
// Allow address to be right behind section ('exclusive end address') since copy size is 0
boolean isZeroSized = true;
return verifyValidAddress(srcAddress, isZeroSized) && verifyValidAddress(destAddress, isZeroSized);
}

lock.writeLock().lock();
try {
sectionsMap.performCopyAccess(srcAddress, destAddress, bytesCount);
Expand Down
7 changes: 7 additions & 0 deletions agent-impl/src/main/java/marcono1234/unsafe_sanitizer/agent_impl/UnsafeSanitizerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,13 @@ public static boolean onWriteAccess(@Nullable Object obj, long address, long byt

private static boolean onAccess(@Nullable Object obj, long address, long bytesCount, boolean isRead) {
if (obj == null) {
// If `bytesCount` is 0, allow address 0; this assumes that user code might first call `Unsafe#allocateMemory`
// and then uses the returned address as argument to `copyMemory` or `setMemory`;
// if the bytes count is 0, then `allocateMemory` will return 0 as address
if (bytesCount == 0 && address == 0) {
return true;
}

var memoryTracker = getMemoryTracker();
return memoryTracker == null
|| memoryTracker.onAccess(address, bytesCount, isRead);
Expand Down
108 changes: 106 additions & 2 deletions src/test/java/marcono1234/unsafe_sanitizer/UnsafeThrowErrorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,29 @@ void copyMemoryArray() {
unsafe.copyMemory(a, from, a, to, 2 * Long.BYTES);
assertArrayEquals(new long[] {1, 2, 1, 2}, a);
});

assertNoBadMemoryAccess(() -> {
byte[] a = {1};
byte[] b = {0};
// Copy of size 0 should have no effect
unsafe.copyMemory(a, ARRAY_BYTE_BASE_OFFSET, b, ARRAY_BYTE_BASE_OFFSET, 0);
assertArrayEquals(new byte[] {1}, a);
assertArrayEquals(new byte[] {0}, b);

// Copy of size 0 should also be allowed right behind last array element, i.e. at 'exclusive end address' (but should have no effect)
long endOffset = ARRAY_BYTE_BASE_OFFSET + a.length * ARRAY_BYTE_INDEX_SCALE;
unsafe.copyMemory(a, endOffset, b, ARRAY_BYTE_BASE_OFFSET, 0);
unsafe.copyMemory(b, ARRAY_BYTE_BASE_OFFSET, a, endOffset, 0);
assertArrayEquals(new byte[] {1}, a);
assertArrayEquals(new byte[] {0}, b);

// Size 0 should allow using address 0 (should have no effect)
unsafe.copyMemory(a, ARRAY_BYTE_BASE_OFFSET, null, 0, 0);
unsafe.copyMemory(null, 0, b, ARRAY_BYTE_BASE_OFFSET, 0);
assertArrayEquals(new byte[] {1}, a);
assertArrayEquals(new byte[] {0}, b);
});

assertBadMemoryAccess(() -> {
byte[] b = {1, 2, 3, 4};
long from = ARRAY_BYTE_BASE_OFFSET;
Expand All @@ -575,6 +598,28 @@ void copyMemoryArray() {
unsafe.copyMemory(b, from, b, to, 2);
});

{
byte[] a = {1};
// Should fail for invalid address, even if size is 0
long maxOffset = ARRAY_BYTE_BASE_OFFSET + a.length * ARRAY_BYTE_INDEX_SCALE;
var e = assertBadMemoryAccess(() -> unsafe.copyMemory(a, ARRAY_BYTE_BASE_OFFSET, a, maxOffset + 1, 0));
assertEquals("Bad array access at offset " + (maxOffset + 1) + ", size 0; max offset is " + maxOffset, e.getMessage());
e = assertBadMemoryAccess(() -> unsafe.copyMemory(a, ARRAY_BYTE_BASE_OFFSET, null, -1, 0));
assertEquals("Invalid address: -1", e.getMessage());
e = assertBadMemoryAccess(() -> unsafe.copyMemory(null, -1, a, ARRAY_BYTE_BASE_OFFSET, 0));
assertEquals("Invalid address: -1", e.getMessage());
}

{
byte[] b = {1};
// Should fail for array offset 0 (assuming that base offset != 0), even if size is 0
assert ARRAY_BYTE_BASE_OFFSET > 0;
var e = assertBadMemoryAccess(() -> unsafe.copyMemory(b, 0, b, ARRAY_BYTE_BASE_OFFSET, 0));
assertEquals("Bad array access at offset 0; min offset is " + ARRAY_BYTE_BASE_OFFSET, e.getMessage());
e = assertBadMemoryAccess(() -> unsafe.copyMemory(b, ARRAY_BYTE_BASE_OFFSET, b, 0, 0));
assertEquals("Bad array access at offset 0; min offset is " + ARRAY_BYTE_BASE_OFFSET, e.getMessage());
}

var e = assertBadMemoryAccess(() -> {
Object[] a = {"a", "b"};
// `copyMemory` only permits primitive arrays
Expand Down Expand Up @@ -603,6 +648,18 @@ public String toString() {
dummy.a = 1;
var e = assertBadMemoryAccess(() -> unsafe.copyMemory(dummy, offsetA, dummy, offsetB, 2));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());

// Should fail even if size is 0
e = assertBadMemoryAccess(() -> unsafe.copyMemory(dummy, offsetA, dummy, offsetB, 0));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());

// Should fail for offset 0 (assuming that no field is at that offset), even if size is 0
assert offsetA != 0 && offsetB != 0;
byte[] b = {1};
e = assertBadMemoryAccess(() -> unsafe.copyMemory(dummy, 0, b, ARRAY_BYTE_BASE_OFFSET, 0));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());
e = assertBadMemoryAccess(() -> unsafe.copyMemory(b, ARRAY_BYTE_BASE_OFFSET, dummy, 0, 0));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());
}

@Test
Expand All @@ -621,11 +678,18 @@ void copyMemory() {
assertEquals(longValue, unsafe.getLong(a));
assertEquals(0, unsafe.getLong(b));

// Copy of size 0 should also be allowed right behind sections, i.e. at 'exclusive end address'
// Copy of size 0 should also be allowed right behind sections, i.e. at 'exclusive end address' (but should have no effect)
unsafe.copyMemory(null, a + size, null, b + size, 0);
assertEquals(longValue, unsafe.getLong(a));
assertEquals(0, unsafe.getLong(b));

// Size 0 should allow using address 0 (should have no effect)
unsafe.copyMemory(null, 0, null, 0, 0);
unsafe.copyMemory(null, a, null, 0, 0);
unsafe.copyMemory(null, 0, null, b, 0);
assertEquals(longValue, unsafe.getLong(a));
assertEquals(0, unsafe.getLong(b));

unsafe.copyMemory(null, a, null, b, size);
assertEquals(longValue, unsafe.getLong(a));
assertEquals(longValue, unsafe.getLong(b));
Expand Down Expand Up @@ -665,6 +729,12 @@ void copyMemory() {
assertBadMemoryAccess(() -> unsafe.copyMemory(null, a, null, b + 1, size));
assertBadMemoryAccess(() -> unsafe.copyMemory(a, b + 1, size));

// Should fail for invalid address, even if size is 0
var e = assertBadMemoryAccess(() -> unsafe.copyMemory(-1, 0, 0));
assertEquals("Invalid address: -1", e.getMessage());
e = assertBadMemoryAccess(() -> unsafe.copyMemory(0, -1, 0));
assertEquals("Invalid address: -1", e.getMessage());

assertBadMemoryAccess(() -> {
class Dummy {
int i;
Expand Down Expand Up @@ -718,6 +788,17 @@ void setMemoryArray() {
unsafe.setMemory(a, ARRAY_OBJECT_BASE_OFFSET, ARRAY_OBJECT_INDEX_SCALE, (byte) 2);
});
assertEquals("Unsupported class " + Object[].class.getTypeName(), e.getMessage());

e = assertBadMemoryAccess(() -> {
byte[] b = {1, 2, 3, 4};
// Should fail for array offset 0 (assuming that base offset != 0), even if size is 0
assert ARRAY_BYTE_BASE_OFFSET > 0;
unsafe.setMemory(b, 0, 0, (byte) 5);
});
assertEquals(
"Bad array access at offset 0; min offset is " + ARRAY_BYTE_BASE_OFFSET,
e.getMessage()
);
}

@Test
Expand All @@ -737,6 +818,15 @@ public String toString() {
dummy.a = 1;
var e = assertBadMemoryAccess(() -> unsafe.setMemory(dummy, offset, 4, (byte) 2));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());

// Should fail even if size is 0
e = assertBadMemoryAccess(() -> unsafe.setMemory(dummy, offset, 0, (byte) 2));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());

// Should fail for offset 0 (assuming that no field is at that offset), even if size is 0
assert offset != 0;
e = assertBadMemoryAccess(() -> unsafe.setMemory(dummy, 0, 0, (byte) 2));
assertEquals("Unsupported class " + Dummy.class.getTypeName(), e.getMessage());
}

@Test
Expand All @@ -750,10 +840,13 @@ void setMemory() {
unsafe.setMemory(a, 0, (byte) 1);
assertEquals(0, unsafe.getLong(a));

// Size 0 should also be allowed right behind section, i.e. at 'exclusive end address'
// Size 0 should also be allowed right behind section, i.e. at 'exclusive end address' (but should have no effect)
unsafe.setMemory(a + size, 0, (byte) 1);
assertEquals(0, unsafe.getLong(a));

// Size 0 should allow using address 0 (should have no effect)
unsafe.setMemory(0, 0, (byte) 1);

unsafe.setMemory(a + 2, 4, (byte) 0x12);
assertEquals(0x12121212, unsafe.getInt(a + 2));
// Other memory should have remained unmodified
Expand All @@ -767,6 +860,10 @@ void setMemory() {
var e = assertBadMemoryAccess(() -> unsafe.setMemory(-1, 2, (byte) 0));
assertEquals("Invalid address: -1", e.getMessage());

// Should fail for invalid address, even if size is 0
e = assertBadMemoryAccess(() -> unsafe.setMemory(-1, 0, (byte) 0));
assertEquals("Invalid address: -1", e.getMessage());

long size = 8;
long a = allocateMemory(size);

Expand All @@ -779,6 +876,13 @@ void setMemory() {
e.getMessage()
);

// Should fail for invalid address, even if size is 0
e = assertBadMemoryAccess(() -> unsafe.setMemory(a + size + 1, 0, (byte) 4));
assertEquals(
"Access outside of section at " + (a + size + 1) + ", size 0 (previous section: " + a + ", size 8)",
e.getMessage()
);

// Clean up
freeMemory(a);
}
Expand Down

0 comments on commit 220c247

Please sign in to comment.