Skip to content

Various fixes in internal.c#10224

Open
embhorn wants to merge 5 commits intowolfSSL:masterfrom
embhorn:zd21594
Open

Various fixes in internal.c#10224
embhorn wants to merge 5 commits intowolfSSL:masterfrom
embhorn:zd21594

Conversation

@embhorn
Copy link
Copy Markdown
Member

@embhorn embhorn commented Apr 14, 2026

Description

Size checking and code quality issues from report

Fixes zd21594

Testing

None

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@embhorn embhorn self-assigned this Apr 14, 2026
Copilot AI review requested due to automatic review settings April 14, 2026 20:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10224

Scan targets checked: wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10224

Scan targets checked: wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
Comment thread src/internal.c
Copilot AI review requested due to automatic review settings April 14, 2026 21:44
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

src/internal.c:1

  • AddPSKtoPreMasterSecret is declared as returning int, but this introduces a bare return; which is invalid in standard C for non-void functions (and may fail compilation depending on warnings-as-errors). Return an appropriate negative error code here (consistent with surrounding error returns in this function), and ensure the caller path handles it.
    src/internal.c:1
  • This bounds check can overflow if idx is large (e.g., near UINT32_MAX), causing the addition to wrap and potentially bypass the check. Prefer an overflow-safe form like comparing len - idx against the required size (after ensuring idx <= len) to guarantee correctness.
    src/internal.c:1
  • Using ((unsigned int)-1) as a max-value sentinel is less clear than using UINT_MAX from <limits.h>. Switching to UINT_MAX improves readability and avoids relying on a cast-from--1 idiom.
    src/internal.c:1
  • inSz - idx is evaluated with the native types of inSz/idx and then compared against an explicitly unsigned LHS. Even though earlier checks likely ensure idx <= inSz, this mixed signed/unsigned arithmetic can still produce compiler warnings and is easy to regress. Consider casting the RHS to the same unsigned type explicitly (or rewriting as a remaining-bytes variable) to keep the comparison type-safe and clearer.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 15, 2026

MemBrowse Memory Report

No memory changes detected for:

@ColtonWilley
Copy link
Copy Markdown
Contributor

Jenkins retest this please

Copilot AI review requested due to automatic review settings April 15, 2026 22:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
Copy link
Copy Markdown

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10224

Scan targets checked: wolfssl-bugs, wolfssl-compliance, wolfssl-consttime, wolfssl-defaults, wolfssl-mutation, wolfssl-proptest, wolfssl-src, wolfssl-zeroize

Findings: 7
7 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
Comment thread src/internal.c
@embhorn embhorn assigned wolfSSL-Bot and unassigned embhorn Apr 16, 2026
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

Successfully merging this pull request may close these issues.

5 participants