Skip to content
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

Issues found with Coverity #207

Closed
dodys opened this issue Sep 15, 2020 · 12 comments
Closed

Issues found with Coverity #207

dodys opened this issue Sep 15, 2020 · 12 comments

Comments

@dodys
Copy link

dodys commented Sep 15, 2020

Hi,

As part of a Main Inclusion Request to support libonig in Ubuntu, we ran coverity on top of version 6.9.5 and it reported a total of 27 errors.

Could you please take a look at them and let us know in case of any false positives.

Thanks,
Eduardo

@kkos
Copy link
Owner

kkos commented Sep 16, 2020

Thank you for your report.

  • src/regcomp.c:5267 Type: Branch past initialization (PW.BRANCH_PAST_INITIALIZATION)
    I don't think this will cause any problems.

  • src/regexec.c:4414 Type: Uninitialized pointer read (UNINIT)

  • src/regexec.c:4461 Type: Uninitialized pointer read (UNINIT)
    I don't think these things actually happen.
    Condition (sr[i].state == SRS_DEAD) blocks execution.

  • src/regparse.c:7951 Type: Failure to restore non-local value (MISSING_RESTORE)

  • src/regparse.c:8343 Type: Failure to restore non-local value (MISSING_RESTORE)
    It's true that I haven't put the value back in, but I don't think this will cause any problems, as it will eventually result in an error.

  • src/unicode_unfold_key.c:3304 Type: Out-of-bounds access (ARRAY_VS_SINGLETON)
    I don't know what kind of problems this will cause.

  • test/test_regset.c:138 Type: Calling risky function (DC.WEAK_CRYPTO)
    I'm not going to fix this.

@dodys
Copy link
Author

dodys commented Sep 16, 2020

Thanks for your quick reply and analysis!

@setharnold
Copy link

Thanks so much for the quick analysis.

The ARRAY_VS_SINGLETON issue that Coverity found is a false positive in this case. (It's a useful enough check, I've seen other programs do something similar, but fail to do the arithmetic to bound the access as has been done here, so I was pleased to see Coverity check for it.)

Thanks for checking the other issues, some felt plausible and were difficult to tell without indepth knowledge.

kkos added a commit that referenced this issue Sep 20, 2020
kkos added a commit that referenced this issue Sep 20, 2020
kkos added a commit that referenced this issue Sep 20, 2020
kkos added a commit that referenced this issue Sep 20, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
kkos added a commit that referenced this issue Sep 21, 2020
@kkos
Copy link
Owner

kkos commented Sep 28, 2020

I think I've fixed everything except what I said in my previous comment that there was no problem.

@setharnold
Copy link

Excellent, thanks; does cbe9f8b represent a security issue or just a bug?

Thanks

@kkos
Copy link
Owner

kkos commented Sep 30, 2020

I don't know what to answer, but I wasn't doing anything wrong, just exceeding the buffer size, so it's probably a security issue.

@setharnold
Copy link

I've asked MITRE to assign a CVE number for this issue; I've tried to summarize it as best I could, I'll write back if they assign one. Thanks!

@setharnold
Copy link

Use CVE-2020-26159 for the buffer overwrite in cbe9f8b .

Thanks again, kkos.

@kkos
Copy link
Owner

kkos commented Oct 1, 2020

Thank you for letting me know.

@kkos kkos closed this as completed Oct 1, 2020
@bukatlib
Copy link

bukatlib commented Oct 1, 2020

The following part of code in concat_opt_exact function seems to be affected by one-byte buffer overflow.

6232 for (i = to->len; p < end; ) {
6233 len = enclen(enc, p);
6234 if (i + len > OPT_EXACT_MAXLEN) {
6235 r = 1; /* 1:full */
6236 break;
6237 }
6238 for (j = 0; j < len && p < end; j++)
6239 to->s[i++] = *p++;
6240 }

Could someone look at it? Thanks.

kkos added a commit that referenced this issue Oct 1, 2020
@bukatlib
Copy link

bukatlib commented Oct 2, 2020

Thank you for the fix.

kkos added a commit that referenced this issue Oct 20, 2020
@carnil
Copy link

carnil commented Feb 3, 2021

Use CVE-2020-26159 for the buffer overwrite in cbe9f8b .

Thanks again, kkos.

Note the CVE has been rejected in meanwhile.

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

5 participants