pgjdbc/pgjdbc GitHub issues and pull requests (mirror)
help / color / mirror / Atom feed[pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
13+ messages / 2 participants
[nested] [flat]
* [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 08:33 "Sasasu (@Sasasu)" <[email protected]>
0 siblings, 0 replies; 13+ messages in thread
From: Sasasu (@Sasasu) @ 2025-01-22 08:33 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
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?
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 09:02 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-22 09:02 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
Please revert this `writeWrapped` was good here
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 09:03 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-22 09:03 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(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, ..)`
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 09:03 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-22 09:03 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(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
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 09:16 "Sasasu (@Sasasu)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: Sasasu (@Sasasu) @ 2025-01-22 09:16 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java:50)
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.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-22 09:35 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-22 09:35 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
Why is this needed? I think it could be reverted as well.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-23 09:07 "Sasasu (@Sasasu)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: Sasasu (@Sasasu) @ 2025-01-23 09:07 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
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.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-23 10:34 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-23 10:34 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
Why is this `token.length -> len` change needed?
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-24 09:16 "Sasasu (@Sasasu)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: Sasasu (@Sasasu) @ 2025-01-24 09:16 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
`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.
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-24 09:25 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-24 09:25 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
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`?
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-24 09:26 "Sasasu (@Sasasu)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: Sasasu (@Sasasu) @ 2025-01-24 09:26 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
(on pgjdbc/src/main/java/org/postgresql/gss/GSSOutputStream.java)
ok....
gssContext.wrap() will not modify the source input
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-27 14:39 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-27 14:39 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
I attempted yet another review, and it ended up with rewriting the test and fixing `GSSInputStream` logic: https://github.com/pgjdbc/pgjdbc/pull/3500/
^ permalink raw reply [nested|flat] 13+ messages in thread
* Re: [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection
@ 2025-01-28 08:10 "vlsi (@vlsi)" <[email protected]>
11 siblings, 0 replies; 13+ messages in thread
From: vlsi (@vlsi) @ 2025-01-28 08:10 UTC (permalink / raw)
To: pgjdbc/pgjdbc <[email protected]>
Merged in https://github.com/pgjdbc/pgjdbc/pull/3500
^ permalink raw reply [nested|flat] 13+ messages in thread
end of thread, other threads:[~2025-01-28 08:10 UTC | newest]
Thread overview: 13+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-01-22 08:33 [pgjdbc/pgjdbc] PR #3492: fix: ArrayIndexOutOfBounds when write big object into GSS enabled connection "Sasasu (@Sasasu)" <[email protected]>
2025-01-22 09:02 ` "vlsi (@vlsi)" <[email protected]>
2025-01-22 09:03 ` "vlsi (@vlsi)" <[email protected]>
2025-01-22 09:03 ` "vlsi (@vlsi)" <[email protected]>
2025-01-22 09:16 ` "Sasasu (@Sasasu)" <[email protected]>
2025-01-22 09:35 ` "vlsi (@vlsi)" <[email protected]>
2025-01-23 09:07 ` "Sasasu (@Sasasu)" <[email protected]>
2025-01-23 10:34 ` "vlsi (@vlsi)" <[email protected]>
2025-01-24 09:16 ` "Sasasu (@Sasasu)" <[email protected]>
2025-01-24 09:25 ` "vlsi (@vlsi)" <[email protected]>
2025-01-24 09:26 ` "Sasasu (@Sasasu)" <[email protected]>
2025-01-27 14:39 ` "vlsi (@vlsi)" <[email protected]>
2025-01-28 08:10 ` "vlsi (@vlsi)" <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox