Skip to content

fix(citizen-scripting-v8): Crash when formatting an exception stack trace from a callback that's inside an eval() function.#3406

Merged
prikolium-cfx merged 1 commit into
citizenfx:masterfrom
Gogsi:node-eval-callback-crash
May 21, 2025
Merged

fix(citizen-scripting-v8): Crash when formatting an exception stack trace from a callback that's inside an eval() function.#3406
prikolium-cfx merged 1 commit into
citizenfx:masterfrom
Gogsi:node-eval-callback-crash

Conversation

@Gogsi

@Gogsi Gogsi commented May 17, 2025

Copy link
Copy Markdown
Contributor

Goal of this PR

Prevent the FXServer instance from crashing, due to the script name being null when an exception is thrown inside a callback, from inside an eval() function. Not all callbacks work, it might only happen for ones triggered from C++ code. For example the callback of https.get() causes the crash, while the callback from setTimeout() doesn't.

A simple reproduction for the issue:

eval(`
    const https = require('https');
    https.get('https://google.com', res => {
        throw new Error("dfgfgdfgdfg")
    })
`);

A few people reported this crash as if it started happening today, without any changes from their side. Due to the weird nature of eval-ing remotely downloaded code, I have high suspicions (but no proof) that they were using leaked resources with backdoors and the payload was causing the crash. Despite that, the server shouldn't crash from scripts, so the issue should be fixed.

How is this PR achieving the goal

Default the script name to (unknown) if the sourceStr value is null

Before the fix:
image

After the fix: (no crash, just the stack trace is printed)
image

This PR applies to the following area(s)

FiveM, RedM, Server, ScRT: JS

Successfully tested on

FXServer with Node 16 and 22. Haven't tested on the client because I don't know how you could trigger this behavior - https.get isn't available.

Game builds: Tested on 3258, most likely irrelevant

Platforms: Windows

Checklist

  • Code compiles and has been tested successfully.
  • Code explains itself well and/or is documented.
  • My commit message explains what the changes do and what they are for.
  • No extra compilation warnings are added by these changes.

… callback that's inside an eval() function.

Causing an exception inside a callback, inside an eval() function crashes the FXServer, because the script name is null.
@github-actions github-actions Bot added RedM Issues/PRs related to RedM ScRT: JS Issues/PRs related to the JavaScript scripting runtime triage Needs a preliminary assessment to determine the urgency and required action labels May 17, 2025
@Gogsi Gogsi changed the title fix(citizen-scripting-v8): Crash when formatting a stack trace from a callback that's inside an eval() function. fix(citizen-scripting-v8): Crash when formatting an exception stack trace from a callback that's inside an eval() function. May 17, 2025

@radium-cfx radium-cfx 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.

Looks good, thank you!

@radium-cfx radium-cfx added ready-to-merge This PR is enqueued for merging and removed triage Needs a preliminary assessment to determine the urgency and required action labels May 19, 2025
@prikolium-cfx prikolium-cfx merged commit 7173d32 into citizenfx:master May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR is enqueued for merging RedM Issues/PRs related to RedM ScRT: JS Issues/PRs related to the JavaScript scripting runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants