From 92438a6d0b10f182872b1756bf881d842d7c3213 Mon Sep 17 00:00:00 2001 From: Boris Batkin Date: Fri, 12 Jun 2026 00:42:11 -0700 Subject: [PATCH] codeql: fix 4 integer-overflow findings + filter vendored SARIF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL c-cpp alerts on our own code (vendored/generated alerts handled separately by the SARIF filter below): - string_repeat: len*count was a 32-bit multiply feeding allocateString (which takes uint64_t), so repeat(64KB-string, 65537) wrapped to a tiny allocation while the copy loop wrote ~4GB past it — a script-reachable heap overflow. Widen to uint64_t * uint64_t. - dasRaster same-channel memcpy (3 sites): num_pixels*src_ch[*sizeof] multiplied in int32 before promoting to the size_t length; wraps for >2G-element images. Cast operands to size_t. (Only 2 were flagged; the u8 path has the identical shape and is fixed too.) - volume_mixer lookahead index: attack_samples*nChannels was a uint32 multiply inside otherwise-64-bit indexing. Cast to uint64_t. - tutorial 14_passing_arrays.c: printf("%u", owned.size) on a uint64_t field is varargs UB. Use %llu + cast. Workflow: build-mode none does not honor the init paths-ignore for C/C++ results (a tests-cpp/3rdparty/doctest alert survived two master scans with that config), so vendored/generated findings never auto-filtered. Add a github/filter-sarif pass between analyze (upload:false) and upload-sarif, dropping 3rdparty, stb_*.h, sqlite, minfft, and the frozen v1 dasFormatter parser. src/parser generated lexers are NOT excluded — our hand-written rule bodies live there. Dropped results auto-close their alerts on the next master scan. Co-Authored-By: Claude Fable 5 --- .github/workflows/codeql.yml | 26 +++++++++++++++++++++ modules/dasAudio/src/volume_mixer.h | 2 +- modules/dasStbImage/src/dasRaster.cpp | 9 ++++--- src/builtin/module_builtin_string.cpp | 2 +- tutorials/integration/c/14_passing_arrays.c | 2 +- 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index d0949af00e..0b30fc2fa4 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -58,7 +58,33 @@ jobs: - 3rdparty - tests-cpp/3rdparty + # build-mode none does NOT honor the init paths-ignore for C/C++ results + # (a vendored alert under tests-cpp/3rdparty survived two master scans with + # that config). Filter the SARIF post-analysis instead, then upload the + # filtered file. Dropped results auto-close their alerts on the next scan. + # src/parser generated lexers are deliberately NOT excluded — our rule + # bodies live in them (a real OOB write hid there). - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v3 with: category: "/language:c-cpp" + upload: false + output: sarif-results + + - name: Filter vendored / generated paths from SARIF + uses: advanced-security/filter-sarif@v1 + with: + patterns: | + -**/3rdparty/** + -**/stb_*.h + -modules/dasSQLITE/sqlite/** + -modules/dasMinfft/minfft/** + -utils/dasFormatter/ds_parser.cpp + input: sarif-results/cpp.sarif + output: sarif-results/cpp.sarif + + - name: Upload filtered SARIF + uses: github/codeql-action/upload-sarif@v3 + with: + sarif_file: sarif-results/cpp.sarif + category: "/language:c-cpp" diff --git a/modules/dasAudio/src/volume_mixer.h b/modules/dasAudio/src/volume_mixer.h index fa798f76e4..399dc95de2 100644 --- a/modules/dasAudio/src/volume_mixer.h +++ b/modules/dasAudio/src/volume_mixer.h @@ -366,7 +366,7 @@ void ma_limiter_porcess_pcm_frames_any ( ma_limiter * limiter, float * InFames, uint32_t attack_samples = limiter->attack_samples; for ( uint64_t i=0; i!=nFrames; ++i ) { for ( uint32_t j=0; j!=limiter->nChannels; ++j ) { - float lookahead_sample = InFames[i*limiter->nChannels + j + attack_samples*limiter->nChannels]; + float lookahead_sample = InFames[i*limiter->nChannels + j + uint64_t(attack_samples)*limiter->nChannels]; if (fabs(lookahead_sample) * gain[j] > threshold) { float desired_gain = threshold / fabs(lookahead_sample); gain[j] = gain[j] * attack_coeff + desired_gain * (1 - attack_coeff); diff --git a/modules/dasStbImage/src/dasRaster.cpp b/modules/dasStbImage/src/dasRaster.cpp index 573ad1d849..758aa56a74 100644 --- a/modules/dasStbImage/src/dasRaster.cpp +++ b/modules/dasStbImage/src/dasRaster.cpp @@ -233,6 +233,7 @@ namespace das // ---- channel conversion (loops inside each branch) ---- void rast_convert_channels_u8 ( uint8_t * dst, const uint8_t * src, int32_t num_pixels, int32_t src_ch, int32_t dst_ch ) { + if ( num_pixels <= 0 || src_ch <= 0 ) return; // the memcpy fast-path would turn a negative count into a huge size_t if ( src_ch == 1 && dst_ch == 2 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*2] = src[i]; dst[i*2+1] = 255; } } else if ( src_ch == 1 && dst_ch == 3 ) { @@ -258,11 +259,12 @@ namespace das } else if ( src_ch == 4 && dst_ch == 3 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*3] = src[i*4]; dst[i*3+1] = src[i*4+1]; dst[i*3+2] = src[i*4+2]; } } else if ( src_ch == dst_ch ) { - memcpy(dst, src, num_pixels * src_ch); + memcpy(dst, src, size_t(num_pixels) * size_t(src_ch)); } } void rast_convert_channels_u16 ( uint16_t * dst, const uint16_t * src, int32_t num_pixels, int32_t src_ch, int32_t dst_ch ) { + if ( num_pixels <= 0 || src_ch <= 0 ) return; // the memcpy fast-path would turn a negative count into a huge size_t if ( src_ch == 1 && dst_ch == 2 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*2] = src[i]; dst[i*2+1] = 65535; } } else if ( src_ch == 1 && dst_ch == 3 ) { @@ -288,11 +290,12 @@ namespace das } else if ( src_ch == 4 && dst_ch == 3 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*3] = src[i*4]; dst[i*3+1] = src[i*4+1]; dst[i*3+2] = src[i*4+2]; } } else if ( src_ch == dst_ch ) { - memcpy(dst, src, num_pixels * src_ch * sizeof(uint16_t)); + memcpy(dst, src, size_t(num_pixels) * size_t(src_ch) * sizeof(uint16_t)); } } void rast_convert_channels_f32 ( float * dst, const float * src, int32_t num_pixels, int32_t src_ch, int32_t dst_ch ) { + if ( num_pixels <= 0 || src_ch <= 0 ) return; // the memcpy fast-path would turn a negative count into a huge size_t if ( src_ch == 1 && dst_ch == 2 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*2] = src[i]; dst[i*2+1] = 1.0f; } } else if ( src_ch == 1 && dst_ch == 3 ) { @@ -318,7 +321,7 @@ namespace das } else if ( src_ch == 4 && dst_ch == 3 ) { for ( int32_t i = 0; i < num_pixels; ++i ) { dst[i*3] = src[i*4]; dst[i*3+1] = src[i*4+1]; dst[i*3+2] = src[i*4+2]; } } else if ( src_ch == dst_ch ) { - memcpy(dst, src, num_pixels * src_ch * sizeof(float)); + memcpy(dst, src, size_t(num_pixels) * size_t(src_ch) * sizeof(float)); } } diff --git a/src/builtin/module_builtin_string.cpp b/src/builtin/module_builtin_string.cpp index 21b51f8c7f..af394106aa 100644 --- a/src/builtin/module_builtin_string.cpp +++ b/src/builtin/module_builtin_string.cpp @@ -493,7 +493,7 @@ namespace das char * string_repeat ( const char * str, int count, Context * context, LineInfoArg * at ) { uint32_t len = stringLengthSafe ( *context, str ); if ( !len || count<=0 ) return nullptr; - char * res = context->allocateString(nullptr, len * count, at); + char * res = context->allocateString(nullptr, uint64_t(len) * uint64_t(count), at); for ( char * s = res; count; count--, s+=len ) { memcpy ( s, str, len ); } diff --git a/tutorials/integration/c/14_passing_arrays.c b/tutorials/integration/c/14_passing_arrays.c index e9d34bc1a4..a5bc829564 100644 --- a/tutorials/integration/c/14_passing_arrays.c +++ b/tutorials/integration/c/14_passing_arrays.c @@ -121,7 +121,7 @@ int main(int argc, char ** argv) { das_context_eval_with_catch(ctx, fn_fill, args); if (das_context_get_exception(ctx)) { printf("exception in fill_squares\n"); goto done; } - printf("fill_squares(_, 6) -> size=%u, contents=", owned.size); + printf("fill_squares(_, 6) -> size=%llu, contents=", (unsigned long long)owned.size); for (uint32_t i = 0; i < owned.size; i++) { int v = *(int*)das_array_at(&owned, i, sizeof(int)); printf("%s%d", i ? "," : "", v);