Skip to content

Coverity scan UNINIT CWE on pcre2-10.40 #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ljavorsk opened this issue May 19, 2022 · 8 comments
Closed

Coverity scan UNINIT CWE on pcre2-10.40 #121

ljavorsk opened this issue May 19, 2022 · 8 comments

Comments

@ljavorsk
Copy link

Automatic covscan in our workflow has noticed two UNINIT defects on the pcre2-10.40 version.

Covscan logs:

Error: UNINIT (CWE-457):
pcre2-10.40/src/pcre2_jit_compile.c:2607: var_decl: Declaring variable "status" without initializer.
pcre2-10.40/src/pcre2_jit_compile.c:3034: uninit_use_in_call: Using uninitialized element of array "status.store_offsets" when calling "delayed_mem_copy_finish".
# 3032|   SLJIT_ASSERT(cc == ccend && stackptr == stacktop);
# 3033|   
# 3034|-> delayed_mem_copy_finish(&status);
# 3035|   }
# 3036|
Error: UNINIT (CWE-457):
pcre2-10.40/src/pcre2test.c:8513: var_decl: Declaring variable "found" without initializer.
pcre2-10.40/src/pcre2test.c:8586: uninit_use_in_call: Using uninitialized value "*found[k]" when calling "format_list_item".
# 8584|     char buff2[128];
# 8585|   
# 8586|->   format_list_item(found[k], buff1, wantscripts);
# 8587|     x = k + (n+1)/2;
# 8588|     if (x < n) format_list_item(found[x], buff2, wantscripts);
@PhilipHazel
Copy link
Collaborator

I have looked at the second of these, and cannot see any problem. Also, neither valgrind nor gcc sanitize throws an error at runtime. My suspicion is therefore that this is a false positive. I will leave the first one to Zoltan.

@PhilipHazel
Copy link
Collaborator

For a while I was unable to look at Coverity Scan issue details, but now I can do so. I understand why it is throwing up the second of these, but it is definitely a false positive, and I have marked it as such.

@JetXujing
Copy link
Contributor

I tried to confirm the code, and the first one was supposed to be a false positive.

  1. The value of status->store_bases is initialized to -1 in delayed_mem_copy_init.
  2. The value of status->store_bases can be assigned to other values only in delayed_mem_copy_move, and the value of status->store_offsets must also be set.
  3. The prerequisite for using status->store_offsets in delayed_mem_copy_finish is that status->store_bases is not -1.

According to the preceding information, status->store_offsets is not used before initialization.

@ljavorsk
Copy link
Author

ljavorsk commented Apr 18, 2023

Thank you guys for the confirmation.

@PhilipHazel could you please link the commit where you've marked it as FP into this issue?

When you do that, feel free to close this issue as it is resolved

@PhilipHazel
Copy link
Collaborator

Sorry, I'm afraid I don't understand what you are asking me to do. I must have marked it as FP via the Coverity web site - I don't know how to make the linkage you are asking for.

@ljavorsk
Copy link
Author

I thought you used the FP coverity flags in the code itself. Like mentioned here

@PhilipHazel
Copy link
Collaborator

No, the only flags in the code are LCOV (coverage) flags.

@PhilipHazel
Copy link
Collaborator

Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants