[necessarylion]

Safety fixes in the Rust part. One commit per item. B3 was already fixed on main, so it has no commit (see below).

S6: a short server message kills the Dart process

Problem. The backend message parser in native/src/conn.rs read server bytes with plain indexing: be16, be32, columns() (RowDescription), row() (DataRow) and the auth message in login() (&body[4..], data[..4]). There was no bounds check. Also dpg_connect and dpg_submit did len as usize on lengths from Dart.

What it causes. Release builds use panic = "abort". So a short or broken message from the server (or a proxy, or a man in the middle) aborts the whole Dart process, not just the query. A negative length becomes a huge usize, which is undefined behavior in from_raw_parts.

Fix.

• New take() reads n bytes at p with .get() and gives an error when they are not there. be16 and be32 use it and return Result. columns() and row() return Result, and a column name with no end byte is an error. login() uses .get() too. The error is dox_postgres_native: bad message from the server. It fails the connection, like other read errors.
• New bytes(ptr, len) in lib.rs: a negative length or a null pointer is an empty slice. dpg_connect returns -1 for a negative len or root_len. dpg_submit uses bytes().

Tests: short_or_broken_messages_are_errors_not_panics and negative_ffi_lengths_are_refused.

S5 (Rust part): the verifier takes any certificate

Problem. dpg_connect mapped verify 1 to the full check and 2 to the chain check. Any other value meant no check. With no check, Verifier takes any certificate.

What it causes. A wrong or new verify value turns the check off without a sign. A man in the middle can then read the traffic.

Fix. New Check::of(verify): only 0 skips the check (require, prefer, like libpq). 2 is the chain check. Any other value is the full check (verify-full). So a bad value fails closed.

The main part of S5 is in dox-orm (dartondox/dox-orm#23): there useSSL: true now gives verify-full, not require. The Dart side of this package already sends 1 for verify-full and 2 for verify-ca. An explicit sslMode: 'require' still skips the check on purpose, like libpq (eg. Heroku).

Test: an_unknown_verify_value_is_the_full_check.

B3: the native link is never freed (already fixed)

I checked this and it is not a problem on main any more. gone() in native/src/link.rs ends with Arc::decrement_strong_count(self as *const Link). That gives back the reference from Arc::into_raw in dpg_open. It came in with #5. gone() runs once, from the helper thread, and then the helper returns and drops its own clone. So the link is freed when the isolate exits. No change is made for B3.

Checks

• cargo fmt --check, cargo clippy --release --all-targets -- -D warnings, cargo test --release: pass (11 tests).
• dart format, dart analyze --fatal-infos: no issues.
• DRIVER=postgres dart test --concurrency=1 with a local release build: all pass on Postgres 16 (TLS off, like CI).

[necessarylion]

Bot review

Verdict: OK

No blocking problems. The small points below are optional. They can go in this PR or a later one.

1. CHANGELOG.md: there is no entry. The last fix PR (Fix a missed close, and test before every release #7) added one. S6 changes what users see: a broken server message now fails the connection, and the Dart process no longer dies. Add one line under a new "Unreleased" or next-version heading.
2. native/src/link.rs:867 (gone()): it uses stopped.store(true). dox-mysql-native uses if self.stopped.swap(true, AcqRel) { return; }. Today this is safe. Only the helper thread calls gone(), once, and then it returns. The helper also holds its own Arc, so the link is not freed while gone() runs. dpg_run/dpg_submit take the core lock and check stopped. But if a second caller is ever added, a second decrement_strong_count is a double free. The swap guard costs one line. It would also match the MySQL driver.
3. native/src/lib.rs:707 and :728 (dpg_connect): a null cfg with len > 0 now becomes an empty config. So it connects to localhost with default settings, but it should fail. Also, err is not checked for null before copy_nonoverlapping. Dart never sends null today, so this is only hardening. To fix: return -1 when cfg is null, and skip the error copy when err is null.

What I checked

• S6, slicing and indexing: I ran rg '\[[^\]]*\.\.' and an indexing search on native/src. Every site that reads server data is now checked or already safe:
  • be16, be32, columns, row and the auth R message use take() or .get().
  • frame checks p + 5 and p + 1 + len before it slices.
  • error_body and error_code stay inside p < b.len().
  • ssl_answer reads inb[0] only when filled > 0.
  • waits.pop_front().unwrap() comes right after a front_mut() that is Some.
  • No unwrap or expect touches server data.
  • Length math can not overflow on the 5 targets, because they are all 64-bit.
  • The error goes up through messages()/read() as a Fail, so Dart gets an error and nothing panics.
  • A negative value length other than -1 goes to Dart as is. Dart reads any len < 0 as NULL, so this is fine.
• FFI:
  • bytes() makes a null pointer or a negative length an empty slice.
  • dpg_connect refuses negative lengths.
  • dpg_submit uses bytes().
  • parse() of Dart batches was already bounds-checked.
  • Dart throws when dpg_open returns 0, so a 0 link never reaches dpg_run/dpg_submit.
• S5:
  • Dart maps verify-full to sslmode=require with verify 1, and verify-ca to require with verify 2. So these modes can not fall back to plain text.
  • Check::Full uses the rustls webpki verifier, with the server name from host (IP names work too). The roots are sslRootCert when it is set, and the Mozilla and system roots when it is not.
  • require still means libpq require: TLS with no certificate check.
  • The Dart side only ever sends 0, 1 or 2, so this change does not break anything in this package. The real default change is in dox-orm#23, and the PR body says so.
• B3: I agree it was already fixed on main by Run all Postgres work on the isolate thread #5. gone() gives back the Arc::into_raw reference. I found no double free or use-after-free with today's callers (see point 2).
• Tests:
  • short_or_broken_messages_are_errors_not_panics would panic on main, because it indexes out of bounds.
  • negative_ffi_lengths_are_refused would hit undefined behavior on main.
  • an_unknown_verify_value_is_the_full_check would fail on main, because 7 would map to None.
  • There is no test for B3. That is OK, because this PR does not change it.

Results

• cargo fmt --check: pass
• cargo clippy --release --all-targets -- -D warnings: pass
• cargo test --release: 11 passed
• dart format and dart analyze --fatal-infos: no issues
• DRIVER=postgres dart test --concurrency=1 on a fresh Postgres 16 Docker container: 40 passed
• gh pr checks 9: test and all 5 builds pass. Release and publish are skipped, as expected for a PR.