JSON decoding may cause potential issues related to signatures #61

New issue

Open

opened 2024-11-09 12:57:26 +00:00 by lda · 3 comments

lda commented

2024-11-09 12:57:26 +00:00

Contributor

Mostly a Telodendria issue, but the JSON decoder ignores \u0000:

if (codepoint == 0)

This could cause issues with projects like Telodendria, where an event could contain one. Most homeservers would then manage signatures and hashes with the NULL managed, whereas Telodendria may strip it off, thus causing events being dropped due to invalid hashing(and as such, may cause divergence between Telodendria and non-Telodendria homeservers).

A band-aid fix may be to handle NULL as the 2-byte UTF-8 sequence, which would allow it to be stored in a C string properly, but isn't standard UTF-8 behavior (due to overlongs), and could be then processed by faulty tools as a NULL(though the standard behavior would not allow that)
A more long-term solution may be work towards #4 (though such an interface would need to have a way to convert to a C string, which would require us to think about handling NULL codepoints again)

Mostly a Telodendria issue, but the JSON decoder *ignores* `\u0000`: https://git.telodendria.io/Telodendria/Cytoplasm/src/commit/b4841fffaaa8986e849106fff1c3b70952e49e16/src/Json.c#L528 This could cause issues with projects like Telodendria, where an event could contain one. Most homeservers would then manage signatures and hashes *with* the NULL managed, whereas Telodendria may strip it off, thus causing events being dropped due to invalid hashing(and as such, may cause divergence between Telodendria and non-Telodendria homeservers). A band-aid fix may be to handle NULL as the 2-byte UTF-8 sequence, which would allow it to be stored in a C string properly, but isn't standard UTF-8 behavior (due to overlongs), and could be then processed by faulty tools as a NULL(though the standard behavior would *not* allow that) A more long-term solution may be work towards #4 (though such an interface would need to have a way to convert to a C string, which would require us to think about handling NULL codepoints again)

jordan commented

2024-11-09 13:12:42 +00:00

Owner

Good catch. The reason I did this was precisely because C strings are NULL-terminated, so I agree that we should move toward #4 if we really want to solve this issue, as that's really the only true fix.

Why would we need to convert to a C string, beyond printing to a console? I'm legitimately trying to think of a use case when we'd need C strings, but I'm blanking so maybe you are thinking of something I'm not.

My thinking with #4 is that everything that took a C string before would take our new string type, so Cytoplasm would exclusively work with this custom time, and by extension, anything that uses Cytoplasm probably would too.

Good catch. The reason I did this was precisely because C strings are `NULL`-terminated, so I agree that we should move toward #4 if we really want to solve this issue, as that's really the only true fix. Why would we need to convert to a C string, beyond printing to a console? I'm legitimately trying to think of a use case when we'd need C strings, but I'm blanking so maybe you are thinking of something I'm not. My thinking with #4 is that everything that took a C string before would take our new string type, so Cytoplasm would exclusively work with this custom time, and by extension, anything that uses Cytoplasm probably would too.

lda commented

2024-11-09 15:02:18 +00:00

Author

Contributor

Why would we need to convert to a C string, beyond printing to a console?

A main thought may be having to deal with other C projects/libraries (which may not designed around Cytoplasm, or worse, may not even offer a way to take in any other format than a plain C-string)

> Why would we need to convert to a C string, beyond printing to a console? A main thought may be having to deal with *other* C projects/libraries (which may not designed around Cytoplasm, or worse, may not even offer a way to take in any other format than a plain C-string)

jordan commented

2024-11-09 15:17:58 +00:00

Owner

Ah that's a good point. We can certainly have a function that produces a C string for when it's needed, but unfortunately I don't know if there's anything we can do if a NULL shows up in those strings.

Ah that's a good point. We can certainly have a function that produces a C string for when it's needed, but unfortunately I don't know if there's anything we can do if a `NULL` shows up in those strings.