Bug - Fixed Code scanning alert

fronobulax

Developer
Staff member
See https://github.com/kolmafia/kolmafia/security/code-scanning

which says "Uncontrolled data used in path expression" remains unresolved.

I have submitted two different patches to eliminate this warning and the scanner has not liked them.

If anyone else has ideas, let me know or submit a PR.

If anyone can tell me how to run the security scan on a PR before it was committed at least I can stop committing things before I know they will make the scanner happy.

Note that we are suppressing a scanner warning about encryption methodology since we use the encryption KoL expects so we really can't change until KoL does.
 

MCroft

Developer
Staff member
Where do you think it's coming from?

We may have to look at several ways to log in, including ASH/JS commands. For example, I don't think we sanitize KoLmafiaCLI.java's attemptLogin(username)
 

fronobulax

Developer
Staff member
Where do you think it's coming from?

We may have to look at several ways to log in, including ASH/JS commands. For example, I don't think we sanitize KoLmafiaCLI.java's attemptLogin(username)

I don't know. Checking for double dots in close proximity to usage eliminated about 10 other instances of this warning.

I suppose I could pull baseusername out of the File creation and check it explicitly. In which case I would love to know how to invoke the security scanning on a PR manually so I could see whether it was going to work, or not.
 

Ryo_Sangnoir

Developer
Staff member
To run it manually, run it on your fork on GitHub (at `USERNAME/kolmafia`).

On the `main` branch, add `workflow_dispatch:` to the `on:` map at the top of the `codeql-analysis.yml` file (assuming this is the one that runs the security).

Go to `USERNAME/kolmafia/actions/workflows/codeql-analysis.yml`, and you should see a blue bar with the text "This workflow has a workflow_dispatch event trigger." and a dropdown button on the right saying "Run workflow". Select the branch you'd like to run it on and run it.
 

fronobulax

Developer
Staff member
To run it manually, run it on your fork on GitHub (at `USERNAME/kolmafia`).

On the `main` branch, add `workflow_dispatch:` to the `on:` map at the top of the `codeql-analysis.yml` file (assuming this is the one that runs the security).

Go to `USERNAME/kolmafia/actions/workflows/codeql-analysis.yml`, and you should see a blue bar with the text "This workflow has a workflow_dispatch event trigger." and a dropdown button on the right saying "Run workflow". Select the branch you'd like to run it on and run it.

These instructions don't work/make sense. I suspect it is because I have not forked kolmafia. The suggestion was made to work on branches off of main because it was conceptually easier for someone coming from a SVN and central repository model.

I'll revisit my (other) reasons for not forking and try these eventually. Thank you.
 

MCroft

Developer
Staff member
I don't know if this helps you, but here's a thing I did that didn't help me...
  1. Choose Code scanning alerts
  2. Choose an alert
  3. Choose show paths
  4. Browse through the 96 or 114 steps the scanner thinks it takes to get there.
Screen Shot 2021-11-10 at 4.11.22 PM.png

Screen Shot 2021-11-10 at 4.11.31 PM.pngScreen Shot 2021-11-10 at 4.11.41 PM.pngScreen Shot 2021-11-10 at 4.13.11 PM.png
 

heeheehee

Developer
Staff member
I don't know if this helps you, but here's a thing I did that didn't help me...
Did you get to step 79 (request.getFormField("loginName") in LoginRequest)?

I'd suggest either adding some checks right around there, or around Step 83 (top of Preferences.reset(username)).
 

MCroft

Developer
Staff member
Did you get to step 79 (request.getFormField("loginName") in LoginRequest)?
Heck no! 1, 2, 3, 6, 96, I can't find it.

Mostly what I was doing was documenting the tool.

I'd suggest either adding some checks right around there, or around Step 83 (top of Preferences.reset(username)).
We'll have to also cover the CLI and maybe the gCLI login command.

Or find where they start taking the same code path.

But either way, it seems like a wise precaution.

Up where it shows 96 steps in GenericRequest.java is a dropdown that shows some alternate paths to get the same thing. If we close one path and 3 are left, we will know we're on the right track.
 

Ryo_Sangnoir

Developer
Staff member
These instructions don't work/make sense. I suspect it is because I have not forked kolmafia. The suggestion was made to work on branches off of main because it was conceptually easier for someone coming from a SVN and central repository model.

I'll revisit my (other) reasons for not forking and try these eventually. Thank you.
haha oops, forgot that you were a dev >_>

In that case, it's probably best to add `workflow_dispatch:` to the main kolmafia repo. I only recommended the fork because I think you have to have it on the default branch for the blue bar to appear, and to get it on a non-fork you'd have to file a PR and could be waiting a while.

Any job you want to be able to manually run, you can add `workflow_dispatch:` to it, and then you can run it by following the previous instructions.
 

fronobulax

Developer
Staff member
I have also followed the paths ;-)

But I am lazy and several others were nipped in the bud by checking at the point of making the file so I was kind of hoping that was the special sauce.
 

fronobulax

Developer
Staff member
With fear and trembling I believe I have successfully added a manual trigger to the security scanning and it works for any branch based on main after I did so. I'm off to see if I can squash this :)
 

fronobulax

Developer
Staff member
When viewing security scan results you have to fund a pulldown to get the results of a scan against a branch.
 
Top