I got a few small changes to the ticket system, which I grouped into the branch tkt-improvements.

The changes contain:

• I got annoyed changing things in /tktsetup for testing purposes again and again and having to click through to it. To improve I added a submenu to the sub-pages of /tkt_setup that goes back to it, and also made 'Cancel' consistently go to the parent page. (fossil:f5619ef3e0bfa164)
• The old ticket fossil:3bcacd36ff9e8820 asked for showing the discussion so far on the /tktedit page, which I added. (fossil:ff009a8f0e482a86)
• In ticket reports, if you enable multiple rows per ticket (which can be done by naming field with leading underscore), then an empty row is added for each ticket, which is intended as a separator (as indicated by code comment). I changed it to contain a <hr>, so that it is visibly clearer to understand as separator. (fossil:7b08586a41d11f19)
• Ticket fossil:04ba0f70b816cebe asks for linking the version field in /tktview to the respective version. I added TH1-code to the default template, that checks if the content is either a valid tagname or a valid checkin-hash, and then links to it accordingly. (fossil:44bd394f0a618264)
  • EDIT: With the fixes for the injection issue that code now also checks the version-string before even attempting to query the DB. For checkin-hashes that looks for values containing only numbers and a-f, for tags alphanumeric values and underscores (_), dots (.) and hyphen (-). Values in the field not matching these will not get a link.
• Ticket fossil:14bc0ad52d919860 asks to show creation time as well. I am not aware if back at the time the ticket was created the tkt_ctime field already existed, but it does now. So I added it to the default template for new ticket reports and made the column names more clear (fossil:86c1910ffbd1b642), added the creation time to /tktview (fossil:814a417b4d2d1071) and also added human readable ages for both modification time and creation time to /tktview (fossil:8b48a054edda1a17).
• The submenu to delete a ticket report in /rptedit wasn't working. That is because of the CSRF-check (which is a good thing). I removed the submenu to avoid confusion. The functionality to delete a report is still available if you click the button in the form. (fossil:7c91416166a7ecbb)

As these are different things covered, it might be prudent to remove some of these before merging if you object to a specific thing in the list. I just bundled them in one branch, as each seems to small to justify it's own branch.

All of these changes are on the small side, but I think they all add a little quality of life to the ticket system to justify their inclusion.

I tried to implement as much as possible in changeable templates. This means that a lot of these things (like creation date or linking the version field) are customizable for the repository admins. It also means a lot will not show up in repositories until templates are reverted to default.

What do you think, are these changes fit for inclusion in trunk?

If there are no objection I would go on and merge this.

Please adjust source code files for a maximum line length of 80 characters.

I think I fixed all occurences of longer lines I introduced.

SQL Injection - DO NOT MERGE

SQL injection on the $foundin variable. For example, enter a new ticket where the first character of the "Version:" entry is single-quote (U+0027).

Oh, damn. I'll look into it or if I cannot fix it drop that change (which links the foundin-field to appropriate checkins). But let's see if I can catch it first. Thanks for the hint.

Thanks for the fix of the SQL injection. I added quoting for HTML and URL.

In my test repository I created some very wild tags to test this and found no problem after both our changes.

Also I put up a guard that I only call the query if the string matches a whitelist of characters. I wasted a bit of time here with string match, which isn't sufficient. But regexp does work. This guard means my wild tags aren't working anymore, as the code doesn't even look for such a tag, but I think that is fine as few will put in such characters in their tags and this only means that no link will be generated. Also this is only the default TH1 for the ticket-view, admins can change it at their will to allow for more stuff.

The only thing that bugs me, is that I could not properly quote the minus ('-') for the regexp in TH1, so that this isn't allowed in tags for links to be created as well (I allowed for numbers, letters, dot '.' and underscore '_', which I assume may turn up in tags representing versions).

EDIT: Got the minus added to the pattern, but I only managed via unicode escape.

The only thing that bugs me, is that I could not properly quote the minus ('-') for the regexp in TH1, so that this isn't allowed in tags for links to be created as well (I allowed for numbers, letters, dot '.' and underscore '_', which I assume may turn up in tags representing versions).

In regexes, a hyphen can be matched either by having it outside of a [...] block or place it at the start of [-...], where [-] by itself should match a single hyphen.

We use lots of hyphens in tag names, and some repos (like Tcl Core's) exclusively uses hyphens as separators¹.

---

1. ^{^} Presumably, though a handful of Tcl core dev were unable to confirm this for me, because of compatibility with filesystem names on some old systems, like an inability to handle multiple dots in a name.

Thanks. I managed with Unicode-escape, but this is the better solution as the unicode escape hides what is happening (except you memorized character codes, which I don't).

Having a look at that code, I was confused by the \ before [ ] $. Which are probably part of the TH1-escaping and not the regexp itself. Maybe having the pattern in { } is more readable - certainly less escape-y - but someone with profound TH1/TCL knowledge should judge:

set tagpattern {^[-0-9A-Za-z_\\.]+$}

(already contains the hyphen upfront, btw.)

Thanks alot!

I never in my life used TCL, so coming to fossil and having to deal with TH1 is a challenge for me. Sorry for my shortcomings. And thanks for these hints. This quoting makes indeed the intent of the regexp much more clear. I included that in the branch.

The branch now contains fixes for the injection issue. Are there other concerns?

If no objections are raised I would again consider merging into trunk. So please stop me if you see problems.

Now on the th1-taint branch I have implemented enhancements to TH1 that attempt to automatically detect potential XSS and SQL-injection vulnerabilities. The current state of affairs:

1. Within TH1, string and variable values that can potentially be controlled by a non-admin user are marked as "tainted" internally.

2. If you generate HTML text using the "html" TH1 command, or if you run SQL using the "query" TH1 command and the text you output or the SQL you are trying to run is tainted, then a warning message is written to the error log.

   • The original design was to panic. But I thought that might break too much legacy.
   • We could add a "strict-th1" setting or a "strict" TH1 command that forces a panic or other fatal error if a tainted value is used in an unsafe way.

3. I had to make some adjustments to the default ticket configuration because it was using "html" instead of "puts" to output some date/time strings. This is technically safe to do, but the new tainted string mechanism doesn't know that so it was generating lots of warnings.

Here's How You Can Help:

• Test the current branch on your configurations, after enabling error logging. Are you safe as configured? Do you need to make adjustments?
• Suggest new safety checks to install.
• How can we roll out this security enhancement with minimal disruption to legacy deployments?
• Should I merge this to trunk and/or put it on the main server and see what happens?

Perl taint mode, nice!

I am currently visiting family for easter, so my testing might be limited. Will be able to do better in Tuesday.

---

I get the new warning for the repository title on each visited page, I guess the TH1 generating the page header has to be adapted.

---

I got an segfault with the branch which isn't happening on trunk.

I served locally with:

fossil serve --repolist repositories/ --errorlog fossil-test.log

It happens on creating a new ticket, I only add a title and submit and get the error. Errorlog reports:

------------- 2025-04-19 20:58:34 UTC ------------
panic: Segfault during /tktnew in fossil [5d17ced68d]
Backtrace: fossil(+0x182e50) [0x558249cabe50] /usr/lib/libc.so.6(+0x40110) [0x7fdbeaf68110] fossil(+0x219f61) [0x558249d42f61] fossil(+0x110f77) [0x558249c39f77] fossil(+0x1117f3) [0x558249c3a7f3] fossil(+0x113e3c) [0x558249c3ce3c] fossil(+0x110f77) [0x558249c39f77] fossil(+0x1117f3) [0x558249c3a7f3] fossil(+0x2105ca) [0x558249d395ca] fossil(+0x21c0a6) [0x558249d450a6] fossil(+0x185756) [0x558249cae756] fossil(+0x187588) [0x558249cb0588] fossil(+0x18433a) [0x558249cad33a] fossil(+0x2f006) [0x558249b58006] /usr/lib/libc.so.6(+0x2abfc) [0x7fdbeaf52bfc] /usr/lib/libc.so.6(__libc_start_main+0x85) [0x7fdbeaf52cb5] fossil(+0x2f031) [0x558249b58031] (pid 10951)
while in /tktnew
HTTP_HOST=localhost:8080
HTTP_REFERER=http://localhost:8080/test/tktnew
HTTP_USER_AGENT=Mozilla/5.0 (X11; Linux x86_64; rv:137.0) Gecko/20100101 Firefox/137.0
PATH_INFO=tktnew
REMOTE_ADDR=127.0.0.1
REQUEST_METHOD=POST
REQUEST_URI=/test/tktnew
SCRIPT_NAME=/test