Skip to content

RDKB-65597 : [Risk-High] JST (Generic) Security Fuzzing Report - #35

Open
pavankumar464 wants to merge 2 commits into
developfrom
bug/RDKB-65597-H
Open

RDKB-65597 : [Risk-High] JST (Generic) Security Fuzzing Report#35
pavankumar464 wants to merge 2 commits into
developfrom
bug/RDKB-65597-H

Conversation

@pavankumar464

Copy link
Copy Markdown
Contributor

Root Cause: ftell() causing calloc() to request an enormous allocation and trigger an OOM abort.
Recommendation - Check ftell() return value for -1 before using it as allocation size:

Root Cause: strtok() on const/env Memory
Recommendation - Replace destructive strtok() parsing with read-only boundary detection using strchr(), copy the session ID into a local writable buffer, and use that buffer for validation and file lookup.

Root Cause: Session identifier validation can be bypassed, potentially allowing session hijacking.
Recommendation - Check Session IDs length and prefix, it should contain only alphanumeric suffix characters, avoid in-place cookie modification during parsing, and are accepted only if the corresponding session file exists.

Root Cause: ftell() causing calloc() to request an enormous allocation and trigger an OOM abort.
Recommendation - Check `ftell()` return value for `-1` before using it as allocation size:

Root Cause: `strtok()` on const/env Memory
Recommendation - Replace destructive strtok() parsing with read-only boundary detection using strchr(),
copy the session ID into a local writable buffer, and use that buffer for validation and file lookup.

Root Cause: Session identifier validation can be bypassed, potentially allowing session hijacking.
Recommendation - Check Session IDs length and prefix, it should contain only alphanumeric suffix characters,
avoid in-place cookie modification during parsing, and are accepted only if the corresponding session file exists.
Copilot AI review requested due to automatic review settings August 5, 2026 17:13
@pavankumar464
pavankumar464 requested a review from a team as a code owner August 5, 2026 17:13
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

📋 PR Format Reminder

  • Description missing:
    • Reason for change
    • Test Procedure
    • Risks (Low / Medium / High)
    • Priority (P0 / P1 / P2)

Expected:

TICKET-123 : brief description

Reason for change: why
Test Procedure: how to verify
Risks: Low / Medium / High
Priority: P0 / P1 / P2

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Addresses security-fuzzing findings in the JST (Generic) session and file-loading code paths by hardening session ID parsing/validation and preventing erroneous large allocations from ftell() failures.

Changes:

  • Replace destructive strtok() cookie parsing with bounded, read-only parsing and copy validated session IDs into a local buffer before file lookup.
  • Strengthen session ID validation (exact length, required prefix, alphanumeric suffix) and only accept IDs when the corresponding session file exists.
  • Add ftell() negative-return handling before using file size as an allocation size.

Reviewed changes

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

File Description
source/jst_session.c Makes cookie/session ID parsing non-destructive and strengthens session ID validation before using it for session file lookup.
source/jst_internal.c Adds error handling around ftell() to avoid allocating an unintended huge buffer on ftell() failure.

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

Comment thread source/jst_session.c Outdated
Comment thread source/jst_internal.c Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 5, 2026 17:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

source/jst_session.c:148

  • If the cookie is invalid or the session file doesn't exist, session_identifier remains allocated but empty. On a subsequent session_start() call, the if (session_identifier) fast-path will run and may utime("/tmp/") and return success even though no valid session was established. To avoid this inconsistent state (and a leak on repeated failures), free and reset session_identifier before returning false when it's still empty.
               strncpy(session_identifier, parsed_sesid, SESSION_ID_LENGTH);

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.

3 participants