[Bug]: Macro Expansion Vulnerability in ALL_RANGE_OPS and Similar Macros

[abd-770]

Is there an existing issue for this?

• [X] I have searched the existing issues

Environment

- Milvus version:
- Deployment mode(standalone or cluster):
- MQ type(rocksmq, pulsar or kafka):    
- SDK version(e.g. pymilvus v2.0.0rc2):
- OS(Ubuntu or CentOS): 
- CPU/Memory: 
- GPU: 
- Others:

Current Behavior

Static Scan has reported PRE10-C. Wrap multistatement macros in a do-while loop vulnerability for the macros ALL_RANGE_OPS, ALL_COMPARE_OPS, and ALL_ARITH_CMP_OPS defined in the codebase. The issue arises when these macros are used within control structures (e.g., if, else) without proper wrapping, leading to unintended execution of code, which can introduce logical errors and unexpected behavior.

https://github.com/milvus-io/milvus/blob/323400c190c35c5b30e4f0580c4d822b022290e2/internal/core/src/bitset/detail/platform/x86/avx2-inst.cpp#L44-L49

Although these macros work fine currently since we call them directly. However in future, If we plan to use within an if statement or similar control structures without braces, only the first line will be controlled by the if, and the remaining lines will always execute. For example:

if (condition)
   ALL_RANGE_OPS(INSTANTIATE_WITHIN_RANGE_COLUMN_AVX2, int8_t);

It expands into:

if (condition)
    INSTANTIATE_WITHIN_RANGE_COLUMN_AVX2(int8_t, IncInc);
INSTANTIATE_WITHIN_RANGE_COLUMN_AVX2(int8_t, IncExc);
INSTANTIATE_WITHIN_RANGE_COLUMN_AVX2(int8_t, ExcInc);
INSTANTIATE_WITHIN_RANGE_COLUMN_AVX2(int8_t, ExcExc);

Expected Behavior

No response

Steps To Reproduce

No response

Milvus Log

No response

Anything else?

No response

[xiaofan-luan]

@abd-770 hi abd, would mind help us on fixing it?

[alexanderguzhva]

@abd-770 @xiaofan-luan Technically, these macro statements are not used anywhere but inside this particular file and these macros are #undef-ed at the end of the file. But I can add {} in order to avoid Static Scan false alarms. Btw, what is the static scan tool that you're using so that I could verify the outcome?

[yanliang567]

/assign @alexanderguzhva /unassign

[abd-770]

@alexanderguzhva Adding executable statements such as do-while, can throw declaration error when used in global scope. A workaround fix using functions will solve this but overcomplicates the code unnecessarily. If you have an elegant solution, It would be great. Else, if these macros are called within this file only, and in future, these macros aren't called within any control statements, We need not make any changes.

We use contrast security scan for obtaining these vulnerabilities.

[alexanderguzhva]

I see. The issue may be closed, because all these macros were designed as local temporary ones and are not exposed outside of corresponding scopes, followed by #undefs. @abd-770 Please let me know if there is a way to mark these macro definitions in some way to stop triggering false alarms, similar to // clang-format off. Thanks.

[abd-770]

Sure, I think we can close this issue then. In contrast scanner, we don't have option to exclude particular code block but rather we can exclude unwanted files from the scanner. We'll do that from our end.

[yanliang567]

close for comments above