CodeQL alerts

[esabol]

PR #404 and PR #405 fix a couple of CodeQL alerts, I think. After those, only a few CodeQL alerts remain....

The five "Potential use after free" alerts (#42, #43, #44, #45, #46) for libgearman-server/queue.cc lines 172-176 all appear to be wrong. Those lines come after the server.queue.functions= new queue_st(); line and inside the if (server.queue.functions) { ... } block, so I don't see any problem here. I think these should be dismissed as erroneous alerts. Do you agree, @SpamapS ?

The Uncontrolled format string alert around line 626 of libgearman-server/gearmand.cc appears to be legitimate. The alert suggests that, if a user sets the GEARMAND_PORT environment variable to a string with percent characters in it, for example, it could result in a buffer overflow and/or crash the program. I think this could be fixed by removing any percent characters from the buffer, right? I'm not sure that would actually satisfy CodeQL though, but it would satisfy me enough that the alert could be dismissed as handled. What do you think?

[SpamapS]

Agreed, the use after free ones seem like bugs in CodeQL. Maybe we should report them.

[SpamapS]

For the other alert, I think it doesn't matter, because the environment is essentially an administrative function, not a user function. So if they want to put % chars in there and break the daemon who are we to stop them? Anyway, it can probably be refactored to make it more robust to that and remove the footgun.

[esabol]

Agreed, the use after free ones seem like bugs in CodeQL. Maybe we should report them.

Yeah, I'll do that. OK, I have dismissed them as false positives and added comments explaining why.

I think I'll come up with a PR to truncate the GEARMAN_PORT environment variable value at the first % character.

[esabol]

I think I'll come up with a PR to truncate the GEARMAN_PORT environment variable value at the first % character.

I decided to truncate it at the first non-digit character instead since that would also address issue #320. See PR #411. Please review, @SpamapS.

[esabol]

@SpamapS : Please review the latest version of PR #411. Thanks!

[SpamapS]

I will take a look at it tomorrow. Thanks for being patient!

…

On Fri, Sep 20, 2024 at 12:39 PM Ed Sabol ***@***.***> wrote: @SpamapS <https://github.com/SpamapS> : Please review the latest version of PR #411 <#411>. Thanks! — Reply to this email directly, view it on GitHub <#406 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADS6YDIDOE5ZZ2RWD5SSGTZXR2VPAVCNFSM6AAAAABLGSGULWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRUGUYDENJVGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[esabol]

The last CodeQL alert has been fixed, so I'm closing this issue.