cvxgrp / scs

Splitting Conic Solver
MIT License
553 stars 136 forks source link

Out-of-bounds read inside norm_inf #284

Closed jwnimmer-tri closed 3 months ago

jwnimmer-tri commented 3 months ago

Specifications

Description

Out-of-bounds read inside norm_inf. This was a regression introduced by https://github.com/cvxgrp/scs/pull/278.

When running the latest SCS inside Drake under AddressSanitizer, it flags an out-of-bounds read. Investigating the situation, I can confirm that it is a true problem.

Here is a patch that fixes the problem, to help illustrate:

--- src/linalg.c
+++ src/linalg.c
@@ -153,6 +153,9 @@
 */

 scs_float SCS(norm_inf)(const scs_float *a, scs_int len) {
+  if (len <= 0) {
+    return 0;  // Follow the semantics of BLASI(lange) for zero-size array.
+  }
   blas_int bone = 1;
   blas_int blen = (blas_int)len;
   scs_int idx = (scs_int)BLASI(amax)(&blen, a, &bone);

The change in https://github.com/cvxgrp/scs/pull/278 switched to amax instead of lange, but failed to match the lange behavior when len == 0. In that case lange is specified to return zero, but instead #278 accidentally returns the -1th array value, which is undefined.

How to reproduce

~I can extract this from our regression suite if necessary, but hopefully the bug is clear without this.~

Edit: See below.

Output

Here is the backtrace during the error:

ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 8 ...
    #0 0x560df0ec5e80 in _scs_norm_inf external/scs_internal/src/linalg.c:160:10
    #1 0x560df0eed508 in compute_ruiz_mats external/scs_internal/linsys/scs_matrix.c:174:16
    #2 0x560df0eec060 in _scs_normalize_a_p external/scs_internal/linsys/scs_matrix.c:339:5
    #3 0x560df0ed24f1 in init_work external/scs_internal/src/scs.c:884:15
    #4 0x560df0ed0a0e in scs_init external/scs_internal/src/scs.c:1230:7
    #5 0x560df0ed2e8f in scs external/scs_internal/src/scs.c:1242:16
...
jwnimmer-tri commented 3 months ago

Here's the write_data_filename output from the program that was crashing 284.zip.

bodono commented 3 months ago

Thank you for letting me know @jwnimmer-tri ! I will fix this and cut a new release asap.

jwnimmer-tri commented 2 months ago

Works great, thanks for the quick new release!