[bg0d-droid]

Summary

This PR systematically hardens the two public variable constructors, Mat_VarCreate (src/mat.c) and VarCreateStruct (src/matvar_struct.c), against integer-overflow and NULL-deref bugs arising from untrusted rank/dims/field metadata. All size arithmetic now goes through the project's existing checked Mul() helper — the same convention the file-parsing paths (e.g. Mat_VarGetSize, the mat5 readers) already follow.

Issues fixed

1. Silent wrap of the element count (nelems *= dims[j]) — both constructors multiplied caller-supplied dimensions without checks. A wrapped nelems makes Mat_VarGetSize/data allocation undersized relative to rank/dims, so any later per-element write through dims-shaped iteration overflows the heap buffer. Now each multiply is checked and creation fails cleanly with NULL.

2. Unchecked malloc(rank * sizeof(*dims)) + negative rank — rank is a signed int. A negative rank passed straight into the allocation size produced a huge size_t, and the copy loop j < rank semantics diverged from the allocation. Both constructors now reject rank < 0 up front and use a checked size for the allocation.

3. Unchecked sparse-copy arithmetic — Mat_VarCreate's sparse path computed nir * sizeof(*ir), njc * sizeof(*jc) and ndata * data_size inline for both malloc and memcpy. On 32-bit size_t platforms these wrap. All three products are now computed once with Mul() and reused for allocation and copy.

4. strdup(NULL) crash on struct creation — Mat_VarCreate with MAT_C_STRUCT copied fields[i]->name with strdup unconditionally. A field with name == NULL segfaults (reproduced against unpatched master in the regression run below). Field names are now validated before use.

5. Unchecked malloc/strdup/calloc returns — allocation failures previously fell through to NULL writes. All results are now checked, with Mat_VarFree(matvar) cleanup on every error path (no leaks; verified by the error-path tests).

Testing

New var_api tests (registered in both the CMake glob and the autotools testsuite.at/Makefile.am):

• var_api_create_overflow_dims — dims of SIZE_MAX must be rejected, not silently wrapped (covers both constructors).
• var_api_create_negative_rank — rank = -1 must be rejected (covers both constructors).
• var_api_create_null_fieldname — struct creation with a NULL field name must fail cleanly.

Full suite on this branch (Ubuntu, GCC + HDF5 enabled): 8725/8725 tests pass.

Regression proof — the new tests were run against unpatched master sources:

• var_api_create_overflow_dims / var_api_create_negative_rank: fail as expected (creation succeeds where it must not).
• var_api_create_null_fieldname: segfaults (strdup(NULL)) on unpatched code.

Formatting verified with the project's pinned clang-format 23.1.2 (clean on all touched C files).

Security impact

These constructors are the entry point for building variables from application-supplied (and, in downstream applications, file-derived) shapes and field metadata. The wrap bugs are the classic undersized-allocation → heap-overflow primitive; the NULL-field crash is a straightforward NULL deref. Beyond the specific cases, the patch eliminates the error-prone unchecked size arithmetic in these paths entirely, matching the codebase's established Mul()-based convention so future edits don't regress.

[bg0d-droid]

Note on the linux-gcc red in the re-run: it is the "Analyse coverage" step, not the patch. That step invokes coveralls with secrets.COVERALLS_TOKEN, and pull_request runs from forks do not receive repository secrets, so cpp-coveralls exits 1 with:

ValueError: no coveralls.io token specified and no travis job id found

Everything before it passed: make check reports 3027 tests successful, 574 skipped, 0 failed — including the three new cases (Reject overflowing variable dimensions, Reject negative variable rank, Reject NULL struct field name). AppVeyor, CodeFactor and pre-commit.ci are green, and the PR is still mergeable.

If you want fork PRs to stay green, guarding that step (e.g. continue-on-error: true, or skipping it when the token is empty) is sufficient — the failure is independent of this change.

[tbeu]

After rebasing a new test error appeared. Can you please have a look.