Page MenuHomePhabricator

Including donor's first name as a URL parameter.
Closed, ResolvedPublic

Description

Basic Information Section

Brief description

  • We (Fundraising - Email team) would like to have a donor's first name appear on the donate wiki page. A simple example can be seen in the attached screenshot (mockup, not real example).

donor_firstname.png (1×3 px, 336 KB)

Do you have a project/product/program plan or documentation?

  • As part of our en6C (a.k.a Big English) Email Campaign, this would be implemented during our Email3 sends. Asana Project »

Primary Contacts

  • @ppenloglou
  • @nisrael

What Security Team services do you anticipate needing?

I don’t know.

What is the 'go live' date for deployment of this project

If approved, earliest would be for our first Email3 send on November 6, 2023.


Privacy Information Section

Will any sensitive data to be collected, stored or exposed?

Yes. A donor's first name will be appended to a URL. Example: &fname=Panos&
Subsequently, the value of that parameter would be captured and presented within our donate webpage.

Technical Information Section

Do related discussions exist in Phab, on wiki, or in an RFC'?

No.

Technology Stack

Acoustic ESP (Wikitech article)
Donate Wiki

Security Readiness Review Section

Unsure(?)

Code

URLs will exist in HTML Emails we send out via Acoustic ESP.
Custom appeal pages will/might be created that contain JS to capture the parameter from the URL.

Post-deployment

  • @ppenloglou
  • @Pcoombe as our main donate.wiki developer.

Working test environment

Within Acoustic we can send out emails to test contacts or to ourselves to evaluate this feature.

Event Timeline

sbassett subscribed.

I had originally added some Security Team AppSec tags to this, but I think that was too presumptuous. This seems to be purely be a Privacy Engineering request, for now.

Hey @sbassett ! I was wondering if there were any updates regarding this request? Cheers!

Hey @ppenloglou - as noted above, I've tagged Privacy Engineering to review this issue (a team I am not on). You'd want to chat with @JFishback_WMF or @sguebo_WMF most likely about any status updates. Thanks.

Sorry all - we're (Privacy Engineering ) running a little behind on this. Will be looking at it today in our team scrum and will post any updates back here.

Greetings @JFishback_WMF , was wondering if we had any updates regarding this request? Thank you!

@JFishback_WMF or @priv_eng_sync any word on whether we can move forward with this feature. We want to include this in some testing we will be doing in email in the next week or so.

@sbassett Did AppSec review the JS to ensure that any parameters are scrubbed before injecting on the page?

We have concerns that the following code snippet from this feature contains a XSS that might be triggered by a malicious link opened by a user:

    /* Override hiding on mobile */
    #appeal-wrapper {
        display: block !important;
    }
</style>
<div class="appeal-layout">
    <div class="appeal-text">
        <p class="headline"><span id="donor-greeting">S</span>upport Wikipedia, your daily source of trusted knowledge.</p>
        <p class="paragraph">Your support matters! Sustain this great fountain of knowledge built by people, for people. Keep Wikipedia thriving.</p>
    </div>
</div>
<script>
    const urlParams = new URLSearchParams(window.location.search);
    const firstName = urlParams.get("fname");
    if (firstName){
        if(firstName !== 'Wikimedia Supporter'){
            const capitalizedName = firstName.charAt(0).toUpperCase() + firstName.slice(1); //Capitalize name
            document.getElementById('donor-greeting').innerHTML = `${capitalizedName}, s`;
        }
    }
</script>
</html>

If you compare this code to Figure 6 of this Dom-based attack example page it looks very similar and I can't see what would be preventing the attack in this case on first glance:

https://goteleport.com/blog/xss-attacks/

var params = new URLSearchParams(window.location.search);
             p = document.getElementById("content")
             p.innerHTML = params.get("message")
JFishback_WMF claimed this task.

JS was updated to sanitize the name being injected.

JS was updated to sanitize the name being injected.

Cool, can we get a gerrit/gitlab URL or code diff or general summary about the mitigation added to this task? Thanks.

sbassett triaged this task as High priority.Nov 6 2023, 4:54 PM
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett removed a project: RFS.