Subject: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection

From: "Sasasu (@Sasasu)" <[email-masked]>
Date: Wed, 22 Jan 2025 08:33:27 +0000

fix this error stack

```
java.lang.ArrayIndexOutOfBoundsException: null
        at java.security.jgss/sun.security.jgss.GSSContextImpl.unwrap
        at sun.security.jgss.GSSContextImpl.wrap(GSSContextImpl.java:385)
        at org.postgresql.gss.GSSOutputStream.writeWrapped(GSSOutputStream.java:52)
        at org.postgresql.gss.GSSOutputStream.write(GSSOutputStream.java:76)
        at java.io.FilterOutputStream.write(FilterOutputStream.java:97)
        at org.postgresql.core.PGStream.send(PGStream.java:398)
```

`GSSOutputStream` does not handle the buffer switch correctly, when a message is bigger than the GSS buffer, GSSOutputStream will pass the input buffer offset as the GSS buffer offset into GSSContext. The offset is much bigger than the GSS buffer, so will cause ArrayIndexOutOfBounds.

Fix this by adding the buffer switch logic into `GSSOutputStream.write()` and also adding a test to demo how to reproduce this bug.

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing](https://github.com/pgjdbc/pgjdbc/blob/master/CONTRIBUTING.md) document?
* [x] Have you checked to ensure there aren't other open [Pull Requests](../../pulls) for the same update/change?

<!-- You can erase any parts of this template not applicable to your Pull Request. -->

### New Feature Submissions:

1. [x] Does your submission pass tests?
2. [x] Does `./gradlew styleCheck` pass ?
3. [x] Have you added your new test classes to an existing test suite in alphabetical order?

### Changes to Existing Features:

* [x] Not breaking existing behaviour
* [x] Have you added an explanation of what your changes do and why you'd like us to include them?
* [x] Have you written new tests for your core changes, as applicable?
* [x] Have you successfully run tests with your changes locally?

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Wed, 22 Jan 2025 09:02:19 +0000

(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)

Please revert this `writeWrapped` was good here

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Wed, 22 Jan 2025 09:03:01 +0000

(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java:50)

The actual bug was that the parameter was named `b` while the code below was using `.wrap(buf`.
The fix is to use `.wrap(b, ..)`

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Wed, 22 Jan 2025 09:03:48 +0000

(on pgjdbc/src/test/java/org/postgresql/test/gss/GSSOutputStreamTest.java)

Please make sure the failure message includes both buffer length, offset and the len argument

---

From: "Sasasu (@Sasasu)" <[email-masked]>
Date: Wed, 22 Jan 2025 09:16:03 +0000

Ok, I add the `writeWrapped()` back and looks it can avoid one buffer copy.

But the old code still leaks a buffer switch logic, so I added it.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Wed, 22 Jan 2025 09:35:06 +0000

Why is this needed? I think it could be reverted as well.

---

From: "Sasasu (@Sasasu)" <[email-masked]>
Date: Thu, 23 Jan 2025 09:07:21 +0000

ok, you are right.

`len >= buf.length` will ensure `min(buf.length - count, len) == buf.length`. The reaming data will be written in the next call.

I also fix a bug when I write the test code. Inside `writeWrapped` use the input length, not use the `b.length`, `b` may comes from a bigger buffer.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Thu, 23 Jan 2025 10:34:19 +0000

Why is this `token.length -> len` change needed?

---

From: "Sasasu (@Sasasu)" <[email-masked]>
Date: Fri, 24 Jan 2025 09:16:06 +0000

`token.length` may comes from a bigger buffer, `gssContext.wrap` will encode `b[off: off + len]` and return `b` as `token`. if `b` is bigger than `len`, `writeWrapped()` will wirte an incorrect length.

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Fri, 24 Jan 2025 09:25:49 +0000

Remember: we write the data to the `pgOut` stream here, and we should write `length + contents`.
The contents is `token` byte array which is a **new** array, and it has nothing to do with `b`/`buf`.

Why have you replaced `len`?

---

From: "Sasasu (@Sasasu)" <[email-masked]>
Date: Fri, 24 Jan 2025 09:26:33 +0000

ok....

gssContext.wrap() will not modify the source input

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Mon, 27 Jan 2025 14:39:37 +0000

I attempted yet another review, and it ended up with rewriting the test and fixing `GSSInputStream` logic: https://github.com/pgjdbc/pgjdbc/pull/3500/

---

From: "vlsi (@vlsi)" <[email-masked]>
Date: Tue, 28 Jan 2025 08:10:30 +0000

Merged in https://github.com/pgjdbc/pgjdbc/pull/3500