View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009452 | Taler | mechant backend | public | 2025-01-10 13:06 | 2025-01-10 16:24 |
| Reporter | fefe | Assigned To | Christian Grothoff | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | resolved | Resolution | fixed | ||
| Fixed in Version | 1.0 | ||||
| Summary | 0009452: GNUNET_buffer_write_path: does not reject spaces, newlines etc | ||||
| Description | This function is used to construct URLs, like here for example (taler-merchant-httpd_helper.c): 1008 GNUNET_buffer_write_path (&buf, 1009 "/orders"); 1010 GNUNET_buffer_write_path (&buf, 1011 order_id); 1012 if ( (NULL != claim_token) && 1013 (! GNUNET_is_zero (claim_token)) ) 1014 { 1015 /* 'token=' for human readability */ 1016 GNUNET_buffer_write_str (&buf, 1017 "?token="); 1018 GNUNET_buffer_write_data_encoded (&buf, 1019 (char *) claim_token, 1020 sizeof (*claim_token)); 1021 num_qp++; 1022 } GNUNET_buffer_write_path will skip leading slashes but will not escape or filter other invalid or dangerous characters. Of particular interest would be CRLF because that could be used to do header injection. | ||||
| Tags | No tags attached. | ||||
|
|
The function is not intended to be *exclusively* used to construct URLs. It is more like Java's "StringBuffer" class, intended to be something that generally handles text (C-Strings) and grows a buffer as needed as the string is being constructed. Sure, the merchant helper uses it to build URLs, but that's not something libgnunetutil should care about. So *if* we were to do something about this, it would have to be at the caller-side. Now, on the caller-side, we do construct URLs, but again we pass those to libcurl, which AFAIK is the only correct place to protect against header injections. The curl API requires us to set each header (and the main URL) separately, and AFAIK curl does check adequately against header injection in those APIs. Looking at the specific quoted code, I'm not against adding some kind of check against whitespace/CRLF/etc. in the "order_id"; the rest should already always be safe. |
|
|
ee590a73..9f0ac42d ensures order_ids are limited to "a-zA-Z:.-_" |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2025-01-10 13:06 | fefe | New Issue | |
| 2025-01-10 13:06 | fefe | Status | new => assigned |
| 2025-01-10 13:06 | fefe | Assigned To | => Christian Grothoff |
| 2025-01-10 16:14 | Christian Grothoff | Note Added: 0023968 | |
| 2025-01-10 16:24 | Christian Grothoff | Note Added: 0023969 | |
| 2025-01-10 16:24 | Christian Grothoff | Status | assigned => resolved |
| 2025-01-10 16:24 | Christian Grothoff | Resolution | open => fixed |
| 2025-01-10 16:24 | Christian Grothoff | Fixed in Version | => 1.0 |
