Bug 19847 - tracklinks.pl accepts any url from a parameter for proxying
Summary: tracklinks.pl accepts any url from a parameter for proxying
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 21018 20218
  Show dependency treegraph
 
Reported: 2017-12-19 22:00 UTC by Liz Rea
Modified: 2018-06-29 13:14 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19847: Track links within the records and 404 for others (2.14 KB, patch)
2017-12-20 15:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19847 - tracklinks.pl accepts any url from a CGI parameter for proxying (2.78 KB, patch)
2017-12-20 21:05 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 19847: Track links within the records and 404 for others (2.20 KB, patch)
2017-12-26 21:27 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19847: Track links within the records and 404 for others (2.25 KB, patch)
2018-01-05 11:25 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Liz Rea 2017-12-19 22:00:16 UTC
tracklinks.pl will accept any url from a parameter, even ones not in Koha. In the wild I have spotted bots doing nasty things with this, such as proxying porn links through a Koha server, I suppose for generating fake hits to game ad services.

We should only track links that come from within Koha records, and 404 anything else, unless someone has a use case for allowing other links (such as those from the home page/nav blocks maybe?) from Koha to do this. I could see people trying it, but I highly doubt anybody actually would because they've got google etc. analytics for that kind of thing.
Comment 1 Katrin Fischer 2017-12-20 06:42:08 UTC
I think limiting to the links within records makes sense. How would you check that?
Comment 2 Jonathan Druart 2017-12-20 15:16:37 UTC
Created attachment 69949 [details] [review]
Bug 19847: Track links within the records and 404 for others

Test plan:
Behave like a robot, you will get 404
Be a human, you will be tracked
Comment 3 Jonathan Druart 2017-12-20 15:17:27 UTC
Something like that?
Comment 4 Liz Rea 2017-12-20 21:05:56 UTC
Created attachment 69975 [details] [review]
Bug 19847 - tracklinks.pl accepts any url from a CGI parameter for proxying

To replicate:

Disable tracklinks in your system preferences
go to, for example:
http://<your name here>/cgi-bin/koha/tracklinks.pl?uri=http://google.com

This will be passed through. This is the case we want to avoid.

So will this:
http://<your name here>/cgi-bin/koha/tracklinks.pl?uri=http://www.google.com&biblionumber=24
assuming you've got a biblionumber 24. This case is OK.

Test with tracklinks set to track, and anonymous. Click both again. Both should still work.

Apply this patch, and test:

http://<your name here>/cgi-bin/koha/tracklinks.pl?uri=http://google.com
This should not work with tracklinks on any setting - should 404.

http://<your name here>/cgi-bin/koha/tracklinks.pl?uri=http://www.google.com&biblionumber=24
This should still work with tracklinks set to track or anonymous.

Test also that from the OPAC detail and results pages that the links from those pages still work in all settings of tracklinks.
Comment 5 Liz Rea 2017-12-20 21:07:33 UTC
oh drat you just beat me, took a little different approach, I only check we have a record and a biblionumber and don't bother with checking the urls come from the record.

Your way is probably better.
Comment 6 Katrin Fischer 2017-12-26 21:27:05 UTC
Created attachment 70167 [details] [review]
Bug 19847: Track links within the records and 404 for others

Test plan:
Behave like a robot, you will get 404
Be a human, you will be tracked

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Nick Clemens 2018-01-05 11:25:37 UTC
Created attachment 70283 [details] [review]
Bug 19847: Track links within the records and 404 for others

Test plan:
Behave like a robot, you will get 404
Be a human, you will be tracked

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Jonathan Druart 2018-02-15 19:07:51 UTC
Patch pushed to
* master for 18.05
* 17.11.x for v17.11.02
* 17.05.x for v17.05.08