deps: cherry-pick e807d4e379 from SQLite

[junius-sec]

Backport the SQLite session extension fix for malformed changesets that omit
old values for primary-key columns. The upstream fix avoids passing NULL to
sessionBindValue() while applying UPDATE changesets.

This adds a regression test for DatabaseSync#applyChangeset() to verify that
the malformed changeset returns SQLITE_CORRUPT instead of crashing.

Refs: https://sqlite.org/src/info/e807d4e3798efd53
Refs: https://hackerone.com/reports/3736889
Refs: sqlite/sqlite@b869ed6

Tested on Linux x64:

$ python3 configure.py --without-npm --without-lief
$ make -j16
$ python3 tools/test.py --mode=release test/parallel/test-sqlite-session.js
All tests passed.

[nodejs-github-bot]

Review requested:

• @nodejs/security-wg
• @nodejs/sqlite

[louwers]

We should just stick to the regular SQLite release cadence I think.

[junius-sec]

Thanks for reviewing, @louwers.

I opened this PR because the disclosed HackerOne thread suggested that the
upstream fix could be cherry-picked publicly for the next regular releases and
that opening the PR could allow changelog credit.

If the project prefers to wait for the regular SQLite release cadence instead,
I’m happy to defer to that.

[Renegade334]

I'm agnostic towards merging, but we definitely shouldn't be regression testing on sqlite3's behalf, as it'll cause havoc with shared builds.

[junius-sec]

Agreed, thanks. I pushed an update to skip this regression test when Node is
built with shared SQLite, so it only runs against the bundled SQLite copy that
this PR patches.

If you would prefer not to carry this SQLite regression test in Node at all,
I’m happy to remove it.

[Renegade334]

It should be removed, yes. We're not aiming to test the behaviour of sqlite3, just our binding.

[aduh95]

If it wasn't for the fact that the fix that makes the process not crash hasn't been in a SQLite release yet, the added test would not raise any eyebrow, we can find similar tests in e.g.

node/test/parallel/test-sqlite-session.js

Lines 346 to 349 in 8257091

	name: 'Error',
	message: 'bad parameter or other API misuse',
	errcode: 21,
	code: 'ERR_SQLITE_ERROR'

IMO it's not a blocking concern

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (debe2ed) to head (f0cdb8e).
⚠️ Report is 143 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #63525      +/-   ##
==========================================
+ Coverage   90.05%   90.32%   +0.27%     
==========================================
  Files         714      730      +16     
  Lines      225742   234205    +8463     
  Branches    42727    43919    +1192     
==========================================
+ Hits       203285   211555    +8270     
- Misses      14234    14373     +139     
- Partials     8223     8277      +54     

see 116 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/73781/

[atlowChemi]

@aduh95 @junius-sec Why are we manually patching instead of the regular automated updates? how would this affect future automated updates?

[aduh95]

Without this patch, the process crashes with an uncatchable error, which is IMO bad enough. It will not affect future automated updates, next update will as always overwrite the entire directory. Floating patches that have landed upstream is not out of the ordinary

[junius-sec]

@atlowChemi Thanks for the question. The reason for the manual patch is that the current behavior can crash the process with attacker-controlled input, so I think it’s worth fixing now rather than waiting for the next automated update. I also agree with aduh95 that this shouldn’t affect future automated updates, since the next update will overwrite the whole directory anyway.

[nodejs-github-bot]

Landed in d8ac301