Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return abort(403) | ||
| if index: | ||
| file_path = file_path + "." + index | ||
| resp = send_file(file_path) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 9 months ago
To fix the issue, we need to validate the file_path to ensure it does not allow access to unintended files. This can be achieved by:
- Normalizing the
file_pathusingos.path.normpathto remove any..segments or other potentially malicious path components. - Ensuring that the normalized
file_pathis contained within a predefined safe directory (e.g., a root directory for allowed files). - Raising an exception or returning an error response if the
file_pathis invalid or outside the safe directory.
The fix will involve modifying the code in the remote_static_whole function to include these validation steps before calling send_file.
| @@ -6,2 +6,3 @@ | ||
| import requests | ||
| import os | ||
| from flask import ( | ||
| @@ -111,3 +112,12 @@ | ||
| file_path = file_path + "." + index | ||
| resp = send_file(file_path) | ||
|
|
||
| # Define a safe root directory for file access | ||
| safe_root = "/path/to/safe/directory" | ||
|
|
||
| # Normalize the file path and ensure it is within the safe root | ||
| full_path = os.path.normpath(os.path.join(safe_root, file_path)) | ||
| if not full_path.startswith(safe_root): | ||
| return abort(403) | ||
|
|
||
| resp = send_file(full_path) | ||
| return resp |
| return new_resp | ||
|
|
||
|
|
||
| @alignviewers_bp.route("/remote/static/whole", methods=["OPTIONS", "GET"]) |
There was a problem hiding this comment.
Hi again @molucorner! Sorry for the long time with no comments; the tests were failed on this one so we were guessing it was still in progress. So far, this looks like a test with a send-whole-file without range requests for what amounts to like 10-100 Gb alignment files? That would typically not be workable. Or maybe it was intended to send the bai files only? Did you reach the conclusion alamut still does not support range requests, or am I missing something here?
There was a problem hiding this comment.
That's fair! I should have asked more clearly, but I was glad to take a break from it 😅 It actually does send a range request in the header: when I'm testing it out I use a 40 GB file, and it takes about a second or two to load (though that seems to depend specifically on the number of reads at the region you ar elooking at)
There was a problem hiding this comment.
Oh, cool! But for some reason the existing ranged endpoint didn't work? Does Alamut make slightly different requests than htslib/IGV then? Or does it do some preamble check that it can reach the files in entirety first? If you wish I can try to test a little! I'll branch off your branch then to not mess it up! 😊
There was a problem hiding this comment.
Feel free! I don't think there's much to mess up 😁 Alamut doesn't get past the check_session_tracks check for me, but maybe the rest is fine?
|
4 participants
This PR would close issue #1621
Well. This works, at least in my end, which is about all I can say for it
Some explanantions: Alamut takes the path given in the url and adds ".bai" to the end to find the index, and if that path doesn't lead to it, loading the file fails. So that has to be worked around.
While tokens as I understand generally are sent in a header, I don't know if there's anyway to tell Alamut to send a specific header with the request, so with the url it goes... Also, an ampersand in the path gets interpreted the end of the path, so I can't get it to take both a filename and a seperate token.
Functionally, the biggest downside is that Alamut displays the path above the alignment, so when the path is encoded, that name becomes less than helpful.
Any suggestions?
OR
Testing on cg-vm1 server (Clinical Genomics Stockholm)
Prepare for testing
scout-stageand the server iscg-vm1.ssh <USER.NAME>@cg-vm1.scilifelab.sesudo -iu hiseq.clinicalssh localhostpodman pssystemctl --user stop scout@<name_of_currently_deployed_branch>systemctl --user start scout@<this_branch>systemctl --user status scout.targetscout-stage) to be used for testing by other users.Testing on hasta server (Clinical Genomics Stockholm)
Prepare for testing
ssh <USER.NAME>@hasta.scilifelab.seus; paxa -u <user> -s hasta -r scout-stage. You can also use the WSGI Pax app available at https://pax.scilifelab.se/.conda activate S_scout; pip freeze | grep scout-browserbash /home/proj/production/servers/resources/hasta.scilifelab.se/update-tool-stage.sh -e S_scout -t scout -b <this_branch>us; scout --versionpaxaprocedure, which will release the allocated resource (scout-stage) to be used for testing by other users.How to test:
Expected outcome:
The functionality should be working
Take a screenshot and attach or copy/paste the output.
Review: