Check file path prefixes #1844
Check file path prefixes #1844
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1844 +/- ##
==========================================
- Coverage 79.57% 79.56% -0.02%
==========================================
Files 519 519
Lines 40774 40791 +17
==========================================
+ Hits 32447 32456 +9
- Misses 8327 8335 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -446,14 +450,17 @@ def self.sync_tool_config() | |||
| def self.save_tool_config(scope, tool, name, data) | |||
| json = JSON.parse(data, :allow_nan => true, :create_additions => true) | |||
| config_path = "#{OPENC3_LOCAL_MODE_PATH}/#{scope}/tool_config/#{tool}/#{name}.json" | |||
| return unless File.expand_path(config_path).start_with?(OPENC3_LOCAL_MODE_PATH) | |||
There was a problem hiding this comment.
We're building this so isn't it always going to start with OPENC3_LOCAL_MODE_PATH? Or are we protecting from someone adding ../../../ or whatever in the scope, tool, or name portion?
There was a problem hiding this comment.
Also we should implement these same checks in local_mode.py
There was a problem hiding this comment.
Correct, this is better protection against traversing up the file tree. Basic attacks will be caught by this, but calling expand_path and making sure that it starts in the correct dir is a better way to protect against theoretically more sophisticated attacks
Ah I see, I missed part of local_mode.py, I'll update that... Is there a reason most of that file is commented out? Can we remove the large blocks of commented out code?
There was a problem hiding this comment.
Anything that's part of the plugin installation process can be removed because that's Ruby only. So yeah you can remove it all.
|
No description provided.