If I see code along the lines of:
const ref = application + ':' + endpoint
then I'm going to request that you provide a rigorous proof that neither of the strings application or endpoint can ever contain a colon. In accordance with my belief that code should be obviously correct, not provably correct, your proof will look something like this:
if (application.includes(':') || endpoint.includes(':')) {
throw Error()
}
This is because I have been down this road too many times.
There are two major scenarios where concatenating strings together in this way is dangerous. The first is if we intend to use ref as some kind of unique identifier. A key in a hash table, maybe. In this case, we are at risk of collisions, e.g. application = 'a:b'; endpoint = 'c' collides with application = 'a'; endpoint = 'b:c' as both result in ref = 'a:b:c'.
The second is if we are trying to send two strings through some kind of field which nominally only allows a single string to be passed, with the intention of splitting the single string to recover the two original strings at the other end. In this case, we risk not being able to recover the original strings, e.g. 'a:b:c' could resolve to either application = 'a:b'; endpoint = 'c' or application = 'a'; endpoint = 'b:c', or even:
const [application, endpoint] = ref.split(':')
// application = 'a', endpoint = 'b'
This is particularly amusing if instead of application and endpoint we have, say, username and role. It's all fun and games until someone figures out how to inject username = 'horse:admin'. Yes, I have seen this.
If it is unavoidable application and/or endpoint must allow colons, then see if you can just send them separately. Otherwise, you need a rigorous procedure for properly escaping the delimiter prior to concatenation. And in the second case, unescaping them and recovering the originals.
Discussion (6)
2025-02-03 16:54:43 by trainbrain27:
2025-02-03 16:55:44 by qntm:
2025-02-04 00:04:56 by Andrew:
2025-02-04 00:08:09 by tyler:
2025-02-04 03:27:27 by lalaithion:
2025-02-04 17:06:35 by Toph: