CodeQL is reporting "Security" issues in Jamulus source

[pljones]

Describe the bug

CodeQL has been reporting the following "Security" issues in src/client.cpp:

Multiplication result converted to larger type High
# 18 opened January 10, 2021 05:44 • Detected by CodeQL in src/client.cpp:1314 main
Multiplication result converted to larger type High
# 17 opened January 10, 2021 05:44 • Detected by CodeQL in src/client.cpp:1268 main
Multiplication result converted to larger type High
# 16 opened January 10, 2021 05:44 • Detected by CodeQL in src/client.cpp:1260 main

which were closed as "Won't fix". Re-opening this as an issue.

To Reproduce

View the "Security" tab, "Code scanning alerts"->"View alerts" with the "open" filter removed.

Expected behavior

We shouldn't have any type "High" security defects.

Every build should report the CodeQL security defects in Jamulus src clearly.

[ann0see]

Agree. Security issues must be taken seriously! It's a numeric overflow - maybe we need to use a greater immediate type?

[pljones]

Every build should report the CodeQL security defects in Jamulus src clearly.

Currently, the CodeQL process appears to kick off only when a "build all targets" autobuild runs.

I've not yet understood when the Security tab picks the results up - it appears there's a configuration somewhere that's related to the above, but I'm not sure how.

@hoffie if you're around, can you remember?

[softins]

I've pushed a branch to my repo that I think should fix this easily. See softins@e12c000

It compiles without warnings and doesn't appear to flag the branch under Security.

I can raise it as an alternative PR to #3162 if @pljones is happy.

[softins]

In fact, an even better solution is to completely remove the multiplication from the index, and just step an index by the multiplication factor. See softins@a3885ac

I can raise this as a PR instead if @pljones is happy.

[pljones]

Yep, looks good.