reserve room for nul terminator in checkPathList result buffer

[netliomax25-code]

Found this with ASan while exercising the launcher's .ee path-list handling. checkPathList sizes result as malloc(strlen(pathList)) but the recombined string also needs the trailing nul (plus the separators it reinserts), so a value like /a:/b writes one _TCHAR past the allocation. concatPaths just below already does length + 1; the same applies here.

[Copilot]

Pull request overview

Fixes a heap buffer overflow in the launcher’s path-list handling by ensuring checkPathList() allocates space for the trailing NUL terminator when building its recombined result string.

Changes:

• Adjust checkPathList() result buffer allocation to include space for the terminating NUL.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[netliomax25-code]

Good point. Folded the + 1 into bufferLength so the variable now tracks the actual allocation size, and the realloc branch already uses bufferLength * sizeof(_TCHAR), so capacity reasoning stays consistent throughout. Re-ran the /a:/b case and a couple multi-segment lists under ASan and there's no overflow.

[netliomax25-code]

Added the missing Signed-off-by and force-pushed, so the ECA check should clear now. The Code Analysis job looks like it's failing on a releng step that re-tags the native binaries (fatal: tag 'LBv1-1922' already exists), which seems unrelated to this one-line allocation fix.

[netliomax25-code]

Amended the commit so the author and sign-off match my Eclipse account identity and re-pushed, but the ECA check is still red, so the remaining piece is the agreement on my Eclipse Foundation account rather than the commit metadata. Getting that signed on my end; the patch itself is unchanged.

[github-actions]

Test Results

  234 files  ±0    234 suites  ±0   47m 17s ⏱️ - 3m 59s
2 231 tests ±0  2 182 ✅ ±0   49 💤 ±0  0 ❌ ±0 
6 456 runs  ±0  6 343 ✅ ±0  113 💤 ±0  0 ❌ ±0 

Results for commit 492681b. ± Comparison against base commit 8294c6b.

♻️ This comment has been updated with latest results.

[netliomax25-code]

Rebased onto current master and re-pushed. The ECA account is signed now, so the eclipsefdn/eca check went green on this push (it had been stuck on a stale evaluation from before the agreement was on file).

The still-red checks look unrelated to this one-line launcher fix: Configuration Admin TCK and Code Analysis are also failing on master and on other open PRs right now, and the Jenkins pr-head run shows as aborted rather than a real failure. The GitHub Build Linux/Windows/MacOS jobs and CodeQL are green. Happy to rebase again if you'd like a fresh run once master's TCK is back to green.

[Copilot]

🟢 Approval recommended

The focused allocation fix is correct and resolves the reported overflow without introducing regressions.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[netliomax25-code]

any update?

[laeubi]

@netliomax25-code code change looks harmless to me on its own (so even if +1 would not bee needed here). But builds are failing (maybe infrastructure?) I would suggest that you rebase on latest master and we see if that fixes the problem.

If no one else plans to object I would then say we can just merge the PR as it is minimal and looks viable to me even though it seems not really possible to test it directly (what might be why you get less response here).

[netliomax25-code]

Rebased onto current master (now sitting on top of 8294c6b) and force-pushed, so there should be a fresh run.

On it not really being testable directly, that is fair, there is no harness for the launcher C sources in-tree. What I ran out-of-tree:

1. Compiled eclipseUtil.c with the cocoa launcher's own flags (-Wall -Werror, JDK 21 headers). Clean, no new warnings.
2. Ran the checkPathList body verbatim under ASan with checkPath stubbed two ways: returning its input unchanged (the absolute-path case, which never trips the realloc guard) and returning a longer programDir-prefixed copy (which does trip it). Before the change, /a:/b aborts on a 3-byte write starting at the end of the 5-byte allocation. After it, all nine inputs pass clean and the realloc capacity math still holds.

If it comes back red on something unrelated I can re-push later.

[netliomax25-code]

Results are in, and the rebase did clear the earlier build failures:

1. Build Linux, Build Windows and Build MacOS are all green, as are CodeQL, the service version check, the tck job and the ECA check.
2. The one red check left is Configuration Admin TCK, and it looks environmental rather than related to this change. It is a single test, CMCoordinationTestCase.testCoordinatedConfigurationOnBeforeRegisteredManagedService (1 fail / 142 pass), and the exact same test fails on escape message in built-in error page #1303 and size shared-memory id buffer for 64-bit handle on win64 #1304, which touch unrelated Java files. The check's own comparison against the pre-rebase commit 5b97045 also reports no change in failure count, so the rebase neither caused nor fixed that one.
3. The Eclipse Jenkins job (Code Analysis / pr-head) is still building at the moment.

Nothing in the diff changed, it is still the one-line allocation size.