I just saw in the source of my message that it's easy to read/change the query parameters like nid, mail and url.
So:
1/ If people forward this email to friends the email address will stay readable to all others
2/ If people want to fool your statistics the can alter any of the parameters
Proposal:
1/ Do a symmetric encoding on the parameters so they don't have a meaning (i used RC4 in the past for doing this, see http://en.wikipedia.org/wiki/RC4)
2/ Add a hash so you'll know someone tried to alter the query string
In the end you'll get url's like this
http://example.com/simplenews/statistics/click?pars=hs55HHdg&hash=Hggsdb
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | simplenews_statistics6.x-1.3.patch | 16.05 KB | Docc |
| #1 | 542536_simplenews_statistics.zip | 4.41 KB | attiks |
Comments
Comment #1
attiks commentedIncluded is an archive with support for encoding the querystring, it's backwards compatible with the current way of sending parameters, but it needs more testing.
All parameters (nid, mail, url) are encrypted using RC4 using the install_time variable as the key
After encoding the parameters are encoded so they can safely be send by email.
A hash is calculated on the parameter, if the hash doesn't match the processing is stopped.
The new function uses two parameters, p and h if they are not available the function looks for the old parameters.
Can someone please test this, if ok i'll make a patch.
Comment #2
Docc commentedhi there,
Im fully aware of the security issues and some patches are already lying around to be commited. Still waiting for some response from the drupal security team.
Though encryption is not one of the changes. Ill check out your alterations, already did some testing on my own witn encrypting and mayby we can come up with a final solution. My main concern is backwards compatibility.
regards
Comment #3
attiks commentedThe code in the archive is backwards compatible, if the new parameters (p and q) around found in the querystring it reverts back to the original implementation.
Comment #4
Docc commentedk some cleanup and other fixes in the issues quee all in this patch (against HEAD).
Im using the simplenews private key for creating the hash.
There is still the problem of faking CTR links. Im thinking of implenting a link table so it can check a ctr link against it.
But then again this only applies to the old newsletters without hash. So it would render useless in the future (the hash would be enough)
If you have any ideas or suggestions for this patch let me know.
Tnx already for this one
Comment #5
Docc commented