Security hardening - #154
Open
cruzzer wants to merge 7 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>
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>
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>
Addresses the second of the two TODOs above the access checks in timesheet_view.php. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
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>
Closed
|
Fwiw, in case it's of use to anyone, this PR has been merged into my fork https://github.com/dalers/timetracker (which also includes minor cosmetic updates and useability improvements). |
validateCheckboxGroupInput() returned early only for empty input, then called count() on whatever was passed. A scalar post such as 'projects=abc' (instead of 'projects[]=...') arrives as a non-empty string and raises a TypeError under PHP 8, and on older PHP would be mis-handled rather than rejected. Reject anything that is not an array. This also closes the same latent issue at the existing callers (project_add.php, template_add.php, template_edit.php) and is a prerequisite for the client/task/user forms now calling this validator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
cruzzer
force-pushed
the
security-hardening
branch
from
September 8, 2026 08:32
68a82e1 to
99c341a
Compare
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
2 participants
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/