Security hardening - #154
Open
cruzzer wants to merge 6 commits into
Open
Conversation
The project and template forms already run posted checkbox arrays through ttGroupHelper::validateCheckboxGroupInput(). Do the same on the client, task and user forms, and cast the ids where the bind records are built. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
Bring the imported client tax and project ids in line with what the interactive forms accept before they are used to build a record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
report.php decided whether to offer the "mark approved", "mark paid" and "assign to invoice" controls, but the handlers that carry those actions out repeated none of those checks. Repeat them where the action is performed, keep the record ids collected from posted field names numeric, and do the same for the timesheet id used in the join that scopes the update. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
Addresses the second of the two TODOs above the access checks in timesheet_view.php. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
getReport() is deliberately context-free so that cron can use it for all orgs, so charts.php has to establish for itself that the selected report is one of the current user's own - the same check reports.php and the notification pages already make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
The user and project lists in a report's WHERE clause arrive as comma-separated strings from several places - a form bean, a stored favorite report, a timesheet - and were interpolated as they came. Put them through one helper that keeps only integers, and that returns a value matching nothing rather than an empty list, so a filter can never widen a query. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RJoU8gYFXVsnBZWyTeKBh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes various security issues in anuko/timetracker.
Since I am not a regular user of timetracker, I created an automated test suite, which appears to have not have broken anything.
I recommend all users to stop using https://timetracker.anuko.com/