Skip to content

Commit 9e00e55

Browse files
mrecachinasCopilot
andauthored
Fix security and correctness issues from #25 Array API (#40)
* Fix security and correctness issues in array API - Fix Py_BuildValue format specifiers: use 'L' for int64_t and 'K' for uint64_t instead of 'i' which silently truncates 64-bit values - Fix signed integer overflow UB in best_within_dist when max_dist is large: use -1 sentinel instead of max_dist+1 - Replace new[]/delete[] with malloc/free + NULL check to prevent uncaught std::bad_alloc from crashing the process - Fix NEON bug: max_dist > 0 should be max_dist >= 0 so that max_dist=0 correctly delegates to the native path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add Python 3.14 to CI build matrix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b02e8a4 commit 9e00e55

3 files changed

Lines changed: 22 additions & 21 deletions

File tree

.github/workflows/pythonpackage.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jobs:
1414
runs-on: ubuntu-24.04
1515
strategy:
1616
matrix:
17-
python_minor: [ "10" , "11" , "12" , "13" ]
17+
python_minor: [ "10" , "11" , "12" , "13" , "14" ]
1818

1919
steps:
2020
- uses: actions/checkout@v4.2.2
@@ -83,7 +83,7 @@ jobs:
8383
strategy:
8484
fail-fast: true
8585
matrix:
86-
cibw_python: [ "cp310", "cp311", "cp312", "cp313" ]
86+
cibw_python: [ "cp310", "cp311", "cp312", "cp313", "cp314" ]
8787
cibw_arch: [ "x86_64", "arm64" ]
8888

8989
steps:
@@ -131,7 +131,7 @@ jobs:
131131
matrix:
132132
cibw_buildlinux: [ manylinux, musllinux ]
133133
cibw_arch: [ "x86_64", "aarch64" ]
134-
cibw_python: [ "cp310", "cp311", "cp312", "cp313" ]
134+
cibw_python: [ "cp310", "cp311", "cp312", "cp313", "cp314" ]
135135

136136
steps:
137137
- uses: actions/checkout@v4.2.2
@@ -183,7 +183,7 @@ jobs:
183183
- name: 🛠 Build Hexhamming Python C extension
184184
run: cibuildwheel
185185
env:
186-
CIBW_BUILD: "cp310-* cp311-* cp312-* cp313-*"
186+
CIBW_BUILD: "cp310-* cp311-* cp312-* cp313-* cp314-*"
187187
CIBW_ARCHS_WINDOWS: "AMD64"
188188
CIBW_TEST_REQUIRES: pytest pytest-benchmark
189189
CIBW_TEST_COMMAND: "pytest -s {project}/test"

src/python_hexhamming.cc

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <cstdlib>
12
#include <cstring>
23
#include <string.h>
34
#define PY_SSIZE_T_CLEAN
@@ -334,7 +335,7 @@ static PyObject * check_bytes_arrays_first_within_dist_wrapper(PyObject *self, P
334335

335336
Py_END_ALLOW_THREADS
336337

337-
return Py_BuildValue("i", ret);
338+
return Py_BuildValue("L", ret);
338339
}
339340

340341
/**
@@ -376,7 +377,7 @@ static PyObject * check_bytes_arrays_best_within_dist_wrapper(PyObject *self, Py
376377
return NULL;
377378
}
378379

379-
int64_t best_dist = max_dist + 1;
380+
int64_t best_dist = -1;
380381
int64_t best_index = -1;
381382

382383
Py_BEGIN_ALLOW_THREADS
@@ -385,23 +386,19 @@ static PyObject * check_bytes_arrays_best_within_dist_wrapper(PyObject *self, Py
385386
uint64_t number_of_elements = big_array_size / small_array_size;
386387
uint8_t* pBig = big_array;
387388
for (uint64_t i = 0; i < number_of_elements; i++, pBig += small_array_size) {
388-
if (ptr__hamming_distance_bytes(pBig, small_array, small_array_size, best_dist - 1) == 0)
389+
int64_t threshold = (best_dist >= 0) ? best_dist - 1 : max_dist;
390+
if (ptr__hamming_distance_bytes(pBig, small_array, small_array_size, threshold) == 0)
389391
continue;
390392
dist = ptr__hamming_distance_bytes(pBig, small_array, small_array_size, -1);
391-
if (dist < best_dist) {
393+
if (best_dist < 0 || (int64_t)dist < best_dist) {
392394
best_dist = dist;
393395
best_index = i;
394396
}
395397
}
396-
397-
// Anything found? If not, set dist to -1
398-
if (best_index == -1) {
399-
best_dist = -1;
400-
}
401398

402399
Py_END_ALLOW_THREADS
403400

404-
return Py_BuildValue("ii", best_dist, best_index);
401+
return Py_BuildValue("LL", best_dist, best_index);
405402
}
406403

407404
/**
@@ -444,7 +441,11 @@ static PyObject * check_bytes_arrays_all_within_dist_wrapper(PyObject *self, PyO
444441
}
445442

446443
uint64_t number_of_elements = big_array_size / small_array_size;
447-
uint64_t *out = new uint64_t[number_of_elements * 2];
444+
uint64_t *out = (uint64_t *)malloc(number_of_elements * 2 * sizeof(uint64_t));
445+
if (out == NULL) {
446+
PyErr_NoMemory();
447+
return NULL;
448+
}
448449
uint64_t *o = out;
449450

450451
Py_BEGIN_ALLOW_THREADS
@@ -465,25 +466,25 @@ static PyObject * check_bytes_arrays_all_within_dist_wrapper(PyObject *self, PyO
465466
PyObject *my_list = PyList_New(0);
466467
if (my_list == NULL)
467468
{
468-
delete[] out;
469+
free(out);
469470
PyErr_NoMemory();
470471
return NULL;
471472
}
472473

473474
for (uint64_t *op = out; op < o; op += 2)
474475
{
475-
PyObject *tup = Py_BuildValue("ii", op[0], op[1]);
476+
PyObject *tup = Py_BuildValue("KK", op[0], op[1]);
476477
if (tup == NULL)
477478
{
478-
delete[] out;
479+
free(out);
479480
Py_DECREF(my_list);
480481
PyErr_NoMemory();
481482
return NULL;
482483
}
483484

484485
if (PyList_Append(my_list, tup) == -1)
485486
{
486-
delete[] out;
487+
free(out);
487488
Py_DECREF(tup);
488489
Py_DECREF(my_list);
489490
PyErr_NoMemory();
@@ -492,7 +493,7 @@ static PyObject * check_bytes_arrays_all_within_dist_wrapper(PyObject *self, PyO
492493
Py_DECREF(tup);
493494
}
494495

495-
delete[] out;
496+
free(out);
496497

497498
return my_list;
498499
}

src/python_hexhamming.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ inline uint64_t hamming_distance_loop_string(const char* a, const char* b, const
555555
}
556556
static uint64_t hamming_distance_bytes__extra(const uint8_t* a, const uint8_t* b,
557557
const uint64_t length, const int64_t max_dist) {
558-
if (max_dist > 0)
558+
if (max_dist >= 0)
559559
return hamming_distance_bytes__native(a, b, length, max_dist); //This is faster on ARMs.
560560
uint64_t difference = 0;
561561
uint64_t i = 0;

0 commit comments

Comments
 (0)