we are moving to using gravityforms in wordpress and it has the option to pass form variables via the url to a confirmation page for our donation page.
I’ve put this together from a couple of different bits I’ve found
<script type="text/javascript">
function sanitize(input) {
return input
.replace(/([^a-z\d\s]+)/gi, ' ')
.replace(/(\s+)/gi, ' ');
}
// Parse the URL parameter
function getParameterByName(name, url) {
if (!url) url = window.location.href;
name = name.replace(/[\[\]]/g, "\\$&");
var regex = new RegExp("[?&]" + name + "(=([^&#]*)|&|#|$)"),
results = regex.exec(url);
if (!results) return null;
if (!results[2]) return '';
return decodeURIComponent(results[2].replace(/\+/g, " "));
}
// Give the parameter a variable name and sanitize
var dynamicContent = sanitize(getParameterByName('donor'));
var dynamicContent2 = sanitize(getParameterByName('amount'));
//Output the text to the page
document.getElementById("formText").innerText = dynamicContent
document.getElementById("formText2").innerText = dynamicContent2
</script>
It all works as expected and I’ve tested adding things like
and it strips out the characters to stop it making a working link etc.
Other than that is there anything that i should do with the above code or will that safely allow me to display a message along the lines of - Dear ‘Donor’ thanks for £’Amount’ from the url variables?
I’d normally do it in PHP but i don’t have access to that in this setup and I’m not great with JS.
Sorry I might being dense here, but doesn’t this need to be done in PHP? — I know you say you don’t have access, but javascript can be switched off in the browser.
Would generally agree, but I’ve just done some testing and if JS is turned off the rest of the donation form doesn’t work anyway so I guess it doesn’t matter as they wouldn’t get to the confirmation page anyway. (I didn’t build the site - but that’s another story)
As long as this is safe to use I’ll have to run with this. Eventually it will be replaced with our new CRM embedded forms so It is only a stopgap for now.
Just having a look through your code. I believe your sanitize regex can be replaced with one regex
/* valid chars include letters, numbers and single spaces */
const invalidCharsRx = /[^a-z\d\s]+|\s{2,}/gi;
function sanitize(str) {
return str.replace(invalidCharsRx, ’ ');
}
There is no need for a capture group and I have added the | pipe or option to it with a match for 2 or more spaces. (I maybe missing a simpler solution)
A bit confused with this one
name.replace(/[\[\]]/g, "\\$&")
// testing
'[[a]]]'.replace(/[\[\]]/g, "\\$&")
// Is this the desired output
// \\[\\[a\\]\\]\\]
The sanitize code is great, seems to work as it should. thanks for that.
with only the sanitize part changed to your code, if i use the following data
?donor=Jimmy[[a]]]&amount=99
It correctly returns ‘Jimmy a’ and ‘99’ and doesn’t output any backslashes or square brackets at all.
The only other thought I had was to add a limit as it is only a first name and amount field, so it should never be that long. UK gov spec says 35chars for single name fields so I think that would be an ok limit. My security concern is that someone could use this page to add additional text/links to make it look like it is something we are offering and send that to other people. Hopefully there is no other security issue i should be aware of and this would prevent the above happening.
I’ve put this together so hopefully this is an ok way to do it, as it works.
function sanitize(str) {
let text = str.replace(invalidCharsRx,'');
let result = text.substring(0, 35);
return result
}
I’ll do some more learning on this as my knowledge is limited on this, but as long as the code above is working and isn’t going to cause any security issues then that is what i need for now.
I’m showing a bit of my ignorance here, but what I would like to see would be some examples of before and afters — test cases. Maybe 10 examples, including edge cases.
That would be helpful in coming up with a tidy solution.
That’s fine I get the point as does Michael O’Reilly. I would still like to see full examples, before, after, what is expected to be extracted etc. — the bigger picture.
Could almost break it down into a TDD exercise.
Fair enough if Noppy feels it is self explanatory or doesn’t want to do that. It’s just my simpleton brain is struggling to picture what sort of urls are going to be dealt with (encoding etc) and what needs to be extracted.
Yeah… Noppy, the point we always round back to is that it’s always easier to specify what you dont want than what is permissible. There’s a lot of characters in the Unicode library… and depending on what you’re trying to deal with, as rpg says, a lot of them are probably fine.
For example, Is a clown emoji going to be allowed? It can’t (to my knowledge) cause anything bad to happen, but maybe you want to deny it.
Brief:
To display some form field entries in the confirmation page from a Gravityforms submission.
Gravityforms passes these as get variables and has no supplied code to output these to a page.
I cannot use PHP.
The likely fields to be output are ‘name’ and ‘amount’ (for the donation form) although others might at times need to be output, although i can only think that perhaps ‘country’ and ‘event type’ are the ones that we might want for things like our event registrations.
We do have a global audience so (as pointed out) we will need to support names with characters.
My main security concern is someone using this confirmation page to create a page with text and links in the url to pretend that we are offering something and lead people to a phishing attack, so I want to remove any characters that would allow that to succeed. I believe that also adding the limit to the string length will help make it significantly harder for someone to add enough text/code to make the page believable.
If I was to stick with only allowing certain characters -
I did a test and added the W class
/* valid chars include letters, numbers and single spaces */
const invalidCharsRx = /[^a-z\d\s\W]+|\s{2,}/gi;
this then allowed
úiñ o'reilly
without a problem.
But I can see some logic in switching it so it disallows specific characters instead, although I am unsure how I’d do that and what characters in particular would prevent any manipulation attempts.
Removing the period will remove that, as all URLs require at least one period (before the TLD).
You may want to remove @ as well, to avoid mail links.
You may want to strip tags in general. Or just the greater-than and less-than signs.
Will it stop someone from sending a text value that says “www dot myscamsite dot com slash phishme”? No. Will it stop them from sending “Email me for your secret free offer - phishmehard at hotmail dot com”? No.
But at some point, it becomes an arms race. If you let a user set any part of the text, someone will try and find a way to use/abuse it, despite any attempt to circumvent.
Yeah that makes sense. I think that as long as it makes it very unlikely that someone would think that is a real offer etc. that would be as much as i can do. In addition the string length limit it would actually cut off at
www dot myscamsite dot com slash ph
So they can’t build out a page with text and links etc that would make sense/be convincing.
Perhaps I am missing something here, but as the URL search string of the confirmation web page will only contain form data entered by the person making a donation, surely there is inherently no risk. To be extra safe in case they have copied and pasted something into the form, I would suggest simply deleting any greater-than and less-than signs.
To display thanks message on confirmation page, you could so something like:
let params = new URLSearchParams(document.location.search);
document.getElementById(“thanks”).innerHTML = “Dear " + params.get(“name”) + " thanks for £” + params.get(“amount”);
I am probably overthinking it but the scenario I had in mind is that the thankyou page can be found by anyone and the url can be amended to include text that is spit out onto the page.
So a scammer could find our thankyou page (without having to fill the form in even) and add in some text, via the url, about how we are doing some offer or prize draw and ‘here is the link or email to get the offer’.
They can then send that out to a million people hoping that someone will look at the page and think ‘well it’s on a genuine website, so it must be genuine’. If someone checks they would find our domain and charity is genuine so no reason not to trust our page.
So by blocking specific characters and adding a length limit, it would be hard to make the page make sense and have live clickable links etc.
At the end of the day any text that a user can add to a page on our site needs to be minimised to only what needs to be there e.g name with max 35char, and cleaned so it does not contain any characters that could allow it to do something that it shouldn’t e.g links, email
Another approach could be to use sessionStorage to transfer the data (or token) from the donation form web page to the confirmation page. The confirmation page would need to open in the same browser tab.
We are kind of stuck with the options that GravityForms allows.
It does have the option to have a confirmation message in the same page on submit, but it is limited to what else you can have on that page. Using this option though does allow the form field entries to be displayed without it having to be picked up by url.
But in some cases we want the page to be more built out and custom, which is where the other option to pass variables via the url comes in.
Seems worth noting that if you are setting the text in the dom using innerText, then you don’t really have to worry about HTML injection. innerText always renders the value as text and will not interpret any HTML tags that may be in the value, it will just render them as is.
Having a length limit and maybe a check for a URL prefix like may still be a good idea though.
