View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0009461Talermechant backendpublic2025-01-16 16:422025-01-16 17:03
Reporterfefe Assigned Tofefe  
PrioritynormalSeverityminorReproducibilityhave not tried
Status feedbackResolutionopen 
Summary0009461: determine_eligible_accounts: no escaping when building URL
Description```
 762 if (TALER_amount_is_zero (&kyc_amount))
 763 GNUNET_asprintf (&payto_kycauth,
 764 "%s%cmessage=KYC:%s",
 765 exchange_account_payto,
 766 (NULL == strchr (exchange_account_payto,
 767 '?'))
 768 ? '?'
 769 : '&',
 770 merchant_pub_str);
 771 else
 772 GNUNET_asprintf (&payto_kycauth,
 773 "%s%camount=%s&message=KYC:%s",
 774 exchange_account_payto,
 775 (NULL == strchr (exchange_account_payto,
 776 '?'))
 777 ? '?'
 778 : '&',
 779 TALER_amount2s (&kyc_amount),
 780 merchant_pub_str);
```

merchant_pub_str comes from GNUNET_STRINGS_data_to_string_alloc so it should be OK, but I'm not so sure about exchange_account_payto. Generally speaking I think abstractions exist to take burden away from the caller. This API doesn't.
What if any of the fields contain a '&'? There should be no injection possibility here. Everything should be urlencoded on demand. That does not hurt if it's not needed but prevents catastrophe if it does.
TagsNo tags attached.

Activities

Christian Grothoff

2025-01-16 17:03

manager   ~0023990

As you say: we know merchant_pub_str was just base32 encoded by us, so it must be fine.
The base exchange_account_payto MUST already be a URI, and we did validate that one extensively when we received it. Plus, it may contain complex payment instructions, we are just expected to append one more argument. Parsing the URI first, and then encoding it again -- what would that accomplish?

Issue History

Date Modified Username Field Change
2025-01-16 16:42 fefe New Issue
2025-01-16 16:42 fefe Status new => assigned
2025-01-16 16:42 fefe Assigned To => Christian Grothoff
2025-01-16 17:03 Christian Grothoff Note Added: 0023990
2025-01-16 17:03 Christian Grothoff Assigned To Christian Grothoff => fefe
2025-01-16 17:03 Christian Grothoff Status assigned => feedback