Skip to content

Commit dbc23ba

Browse files
committed
common: split utf8_check() into permissive and strict variants
Changelog-None
1 parent 1fc01d3 commit dbc23ba

11 files changed

Lines changed: 87 additions & 34 deletions

File tree

common/bolt11.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ static const char *decode_d(struct bolt11 *b11,
212212
return err;
213213

214214
*have_d = true;
215-
b11->description = utf8_str(b11, take(desc), tal_bytelen(desc));
215+
b11->description = utf8_str_text(b11, take(desc), tal_bytelen(desc));
216216
if (b11->description)
217217
return NULL;
218218

common/bolt12_proof.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ struct tlv_payer_proof *make_unsigned_proof_(const tal_t *ctx,
157157
if (note) {
158158
/* Not nul-terminated! */
159159
pptlv->proof_note = tal_dup_arr(pptlv, utf8, note, strlen(note), 0);
160-
assert(utf8_check(pptlv->proof_note, tal_bytelen(pptlv->proof_note)));
160+
assert(utf8_check_text(pptlv->proof_note, tal_bytelen(pptlv->proof_note)));
161161
}
162162

163163
/* Make sure pptlv->fields correctly reflects values */

common/json_param.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ struct command_result *param_escaped_utf8_string(struct command *cmd,
463463
if (ret)
464464
return ret;
465465

466-
if (!utf8_check(*str, strlen(*str)))
466+
if (!utf8_check_text(*str, strlen(*str)))
467467
return command_fail_badparam(cmd, name, buffer, tok,
468468
"should not contain control, format, private-use or unassigned Unicode characters");
469469
return NULL;
@@ -486,7 +486,7 @@ struct command_result *param_utf8_string(struct command *cmd, const char *name,
486486
if (ret)
487487
return ret;
488488

489-
if (!utf8_check(*str, strlen(*str)))
489+
if (!utf8_check_text(*str, strlen(*str)))
490490
return command_fail_badparam(cmd, name, buffer, tok,
491491
"should not contain control, format, private-use or unassigned Unicode characters");
492492
return NULL;

common/json_param.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,9 @@ struct command_result *param_escaped_string(struct command *cmd,
193193
const jsmntok_t *tok,
194194
const char **str);
195195

196-
196+
/* Extract an escaped string (and unescape it), and reject it if it
197+
* contains characters banned from protocol text fields - see
198+
* param_utf8_string() below. */
197199
struct command_result *param_escaped_utf8_string(struct command *cmd,
198200
const char *name,
199201
const char *buffer,

common/test/run-utils-utf8_check.c

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "config.h"
22
#include <assert.h>
3+
#include <ccan/array_size/array_size.h>
34
#include <common/amount.h>
45
#include <common/pseudorand.h>
56
#include <common/setup.h>
@@ -106,47 +107,67 @@ void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNE
106107
static void test_valid(void)
107108
{
108109
assert(utf8_check("hello world", strlen("hello world")));
110+
assert(utf8_check_text("hello world", strlen("hello world")));
109111

110112
{
111113
static const u8 nansensu[] = {
112114
0xe3, 0x83, 0x8a, 0xe3, 0x83, 0xb3, 0xe3, 0x82,
113115
0xbb, 0xe3, 0x83, 0xb3, 0xe3, 0x82, 0xb9
114116
};
115117
assert(utf8_check(nansensu, sizeof(nansensu)));
118+
assert(utf8_check_text(nansensu, sizeof(nansensu)));
116119
}
117120

118121
assert(utf8_check("", 0));
122+
assert(utf8_check_text("", 0));
119123
}
120124

121-
static void test_banned(void)
125+
/* NUL, surrogates and overlong encodings are rejected by ccan/utf8
126+
* itself (utf8_decode()), regardless of category filtering - even
127+
* plain utf8_check() rejects those */
128+
static void test_encoding_banned(void)
122129
{
123130
static const u8 embedded_nul[] = { 'a', 0x00, 'b' };
124131
assert(!utf8_check(embedded_nul, sizeof(embedded_nul)));
132+
assert(!utf8_check_text(embedded_nul, sizeof(embedded_nul)));
133+
}
125134

126-
assert(!utf8_check("a\tb", 3));
127-
135+
/* Codepoints in Unicode categories Cc/Cf/Co/Cn are valid UTF-8
136+
* encoding (plain utf8_check() accepts them - it's used for general
137+
* JSON-RPC input, datastore, which shouldn't be restricted this
138+
* way), but utf8_check_text() must reject them for protocol text
139+
* fields */
140+
static void test_text_banned(void)
141+
{
142+
static const u8 tab[] = { 'a', '\t', 'b' };
128143
static const u8 del[] = { 'a', 0x7f, 'b' };
129-
assert(!utf8_check(del, sizeof(del)));
130-
131144
static const u8 c1_control[] = { 'a', 0xc2, 0x85, 'b' };
132-
assert(!utf8_check(c1_control, sizeof(c1_control)));
133-
134145
static const u8 rtl_override[] = { 'a', 0xe2, 0x80, 0xae, 'b' };
135-
assert(!utf8_check(rtl_override, sizeof(rtl_override)));
136-
137146
static const u8 private_use[] = { 'a', 0xee, 0x80, 0x80, 'b' };
138-
assert(!utf8_check(private_use, sizeof(private_use)));
139-
140147
static const u8 unassigned[] = { 'a', 0xcd, 0xb8, 'b' };
141-
assert(!utf8_check(unassigned, sizeof(unassigned)));
148+
static const u8 *cases[] = {
149+
tab, del, c1_control, rtl_override, private_use, unassigned
150+
};
151+
static const size_t lens[] = {
152+
sizeof(tab), sizeof(del), sizeof(c1_control),
153+
sizeof(rtl_override), sizeof(private_use), sizeof(unassigned)
154+
};
155+
156+
for (size_t i = 0; i < ARRAY_SIZE(cases); i++) {
157+
/* Plain utf8_check() stays permissive - valid encoding */
158+
assert(utf8_check(cases[i], lens[i]));
159+
/* utf8_check_text() rejects the banned category */
160+
assert(!utf8_check_text(cases[i], lens[i]));
161+
}
142162
}
143163

144164
int main(int argc, char *argv[])
145165
{
146166
common_setup(argv[0]);
147167

148168
test_valid();
149-
test_banned();
169+
test_encoding_banned();
170+
test_text_banned();
150171

151172
common_shutdown();
152173
}

common/unicode_category.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
/* This file was generated by devtools/gen-unicode-category.py */
2-
/* Do not modify this file! Modify the script, or regenerate from a new
3-
* UnicodeData.txt, instead. */
1+
2+
/* This file generated by devtools/gen-unicode-category.py */
3+
/* Do not modify this file!!! Modify the script, or regenerate from a new
4+
* UnicodeData.txt, instead */
45
/* Generated from Unicode 17.0.0 UnicodeData.txt */
56

67
#include "config.h"

common/unicode_category.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
1-
/* This file was generated by devtools/gen-unicode-category.py */
2-
/* Do not modify this file! Modify the script, or regenerate from a new
3-
* UnicodeData.txt, instead. */
41

5-
#ifndef LIGHTNING_COMMON_UNICODE_CATEGORY_GEN_H
6-
#define LIGHTNING_COMMON_UNICODE_CATEGORY_GEN_H
2+
/* This file generated by devtools/gen-unicode-category.py */
3+
/* Do not modify this file!!! Modify the script, or regenerate from a new
4+
* UnicodeData.txt, instead */
5+
6+
#ifndef LIGHTNING_COMMON_UNICODE_CATEGORY_H
7+
#define LIGHTNING_COMMON_UNICODE_CATEGORY_H
78
#include "config.h"
89
#include <ccan/short_types/short_types.h>
910
#include <stdbool.h>
1011

1112
bool unicode_is_banned_codepoint(u32 cp);
1213

13-
#endif /* LIGHTNING_COMMON_UNICODE_CATEGORY_GEN_H */
14+
#endif /* LIGHTNING_COMMON_UNICODE_CATEGORY_H */

common/utils.c

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,7 @@ void tal_arr_appendn_(void *p, const void *append TAKES, size_t bytes)
181181
tal_arr_append_bytes(p, append, bytes);
182182
}
183183

184-
/* Check for valid UTF-8 */
185-
bool utf8_check(const void *vbuf, size_t buflen)
184+
static bool utf8_check_(const void *vbuf, size_t buflen, bool reject_banned)
186185
{
187186
const u8 *buf = vbuf;
188187
struct utf8_state utf8_state = UTF8_STATE_INIT;
@@ -196,17 +195,29 @@ bool utf8_check(const void *vbuf, size_t buflen)
196195
need_more = false;
197196
if (errno != 0)
198197
return false;
199-
if (unicode_is_banned_codepoint(utf8_state.c))
198+
if (reject_banned && unicode_is_banned_codepoint(utf8_state.c))
200199
return false;
201200
}
202201
return !need_more;
203202
}
204203

205-
char *utf8_str(const tal_t *ctx, const u8 *buf TAKES, size_t buflen)
204+
/* Check for valid UTF-8 */
205+
bool utf8_check(const void *vbuf, size_t buflen)
206+
{
207+
return utf8_check_(vbuf, buflen, false);
208+
}
209+
210+
bool utf8_check_text(const void *vbuf, size_t buflen)
211+
{
212+
return utf8_check_(vbuf, buflen, true);
213+
}
214+
215+
static char *utf8_str_(const tal_t *ctx, const u8 *buf TAKES, size_t buflen,
216+
bool reject_banned)
206217
{
207218
char *ret;
208219

209-
if (!utf8_check(buf, buflen)) {
220+
if (!utf8_check_(buf, buflen, reject_banned)) {
210221
tal_free_if_taken(buf);
211222
return NULL;
212223
}
@@ -217,6 +228,16 @@ char *utf8_str(const tal_t *ctx, const u8 *buf TAKES, size_t buflen)
217228
return ret;
218229
}
219230

231+
char *utf8_str(const tal_t *ctx, const u8 *buf TAKES, size_t buflen)
232+
{
233+
return utf8_str_(ctx, buf, buflen, false);
234+
}
235+
236+
char *utf8_str_text(const tal_t *ctx, const u8 *buf TAKES, size_t buflen)
237+
{
238+
return utf8_str_(ctx, buf, buflen, true);
239+
}
240+
220241
char *tal_strdup_or_null(const tal_t *ctx, const char *str)
221242
{
222243
if (!str) {

common/utils.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,13 @@ bool utf8_check(const void *buf, size_t buflen);
139139
/* Check it's UTF-8, return copy (or same if TAKES), or NULL if not valid. */
140140
char *utf8_str(const tal_t *ctx, const u8 *buf TAKES, size_t buflen);
141141

142+
/* Like utf8_check(), but also reject codepoints in Unicode general
143+
* categories Cc/Cf/Co/Cn (control, format, private-use, unassigned) */
144+
bool utf8_check_text(const void *buf, size_t buflen);
145+
146+
/* Like utf8_str(), but using utf8_check_text() instead of utf8_check(). */
147+
char *utf8_str_text(const tal_t *ctx, const u8 *buf TAKES, size_t buflen);
148+
142149
/* Strdup, or pass through NULL */
143150
char *tal_strdup_or_null(const tal_t *ctx, const char *str);
144151

wire/fromwire.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ void fromwire_u8_array(const u8 **cursor, size_t *max, u8 *arr, size_t num)
232232
void fromwire_utf8_array(const u8 **cursor, size_t *max, char *arr, size_t num)
233233
{
234234
fromwire(cursor, max, arr, num);
235-
if (!utf8_check(arr, num))
235+
if (!utf8_check_text(arr, num))
236236
fromwire_fail(cursor, max);
237237
}
238238

0 commit comments

Comments
 (0)