Sodium

Sodium

44M Downloads

MixinMatrix4f#writeToBuffer assumes that it is always working with direct buffers

coderbot16 opened this issue ยท 2 comments

commented

The following code takes the address of the FloatBuffer that it has received using MemoryUtil: https://github.com/CaffeineMC/sodium-fabric/blob/4f8646cff6dd6f295fecb4877dbbe8efb1dfdfe0/src/main/java/me/jellysquid/mods/sodium/mixin/core/matrix/MixinMatrix4f.java#L278

MemoryUtil assumes that it is working with direct byte buffers, and uses Unsafe to read the value of the address field from the buffer. However, if this is not a direct buffer, such as one allocated with FloatBuffer.allocate(16), then the value returned from MemoryUtil is not defined (since non-direct FloatBuffers don't have an address field).

At best, this will crash the game, and at worst it will place the contents of the matrix somewhere on the heap.

Recommended fix:

    /**
     * @reason Optimize
     * @author JellySquid
     */
    @Environment(EnvType.CLIENT)
    @Overwrite
    public void writeToBuffer(FloatBuffer buf) {
        if (buf.remaining() < 16) {
            throw new BufferUnderflowException();
        }

-        if (UnsafeUtil.isAvailable()) {
+        if (buf.isDirect() && UnsafeUtil.isAvailable()) {
            this.writeToBufferUnsafe(buf);
        } else {
            this.writeToBufferSafe(buf);
        }
    }
commented

For non-direct buffers, the address field will simply be left uninitialized at a zero address, so the worst-case scenario here is a memory access violation. Regardless, good catch. Thanks.

commented

Ah interesting, I just looked and address is a field of Buffer, which I didn't know before. Good to know that there's no possible memory corruption here. Thanks for the fix!