Can you explain what exactly "bad" happens here that you are trying to protect against?
> # Attack Succeeded. It is equivalent to > # jtag newtap arm soc \ > # -irlen 4 -expected-id 0x4ba00477 > jtag \ > newtap\x00attack \ > arm\x00attack \ > soc\x00attack -\ > irlen\x00attack 4\x00attack000 \ > -expected-id\x00attack 0x4ba00477\x00attack ________________________________ From: Ooi, Cinly <[email protected]> Sent: Friday, July 16, 2021 7:18:21 PM To: OpenOCD <[email protected]> Subject: Dear All While doing security work on the aji_client driver we are developing for openocd, we see the following that looks like it is possible to do a “NULL byte injection” attack on openocd. Having spoken to Antonio, he suggested that I post this to the mailing list. I want to gather opinion on what the community think about this finding. To recap, “NULL byte injection” exploit the fact that the NULL byte is a termination character, so human might be tricked into reading a string like “passwd\00.png” as a PNG file, while in reality, it is looking for the file passwd. The config script below demonstrate the problem: --- # use any driver at hand. adapter driver aji_client; # Attack stopped: invalid command name “jtag” jtag\x00attack \ newtap \ arm soc \ -irlen 4 \ -expected-id 0x4ba00477 # Attack Succeeded. It is equivalent to # jtag newtap arm soc \ # -irlen 4 -expected-id 0x4ba00477 jtag \ newtap\x00attack \ arm\x00attack \ soc\x00attack -\ irlen\x00attack 4\x00attack000 \ -expected-id\x00attack 0x4ba00477\x00attack --- My analysis says when we that openocd will manage to find out that we inserted a NULL byte for the first token of the statement, but not for all subsequent tokens. The difference can be explained by the way the tokens in the statement is parsed: (1) For the first token, parsing is done inside jimtcl/jim.c https://github.com/msteveb/jimtcl/blob/master/jim.c#L4386 Here, it looks like jimtcl do a hashmap lookup to find a handler to interpret the token . The hashmap look up the command handler to interpret the token. However, as it uses the full token as key, e.g. “newtap\x00attack”, it will not be able to match the hash of “newtap” and this negated the attack (2) For all subsequent tokens, the command handler returned in (1) is used. We create the command handler. We have a tendency to use functions provided in src/helper/jim-nvp.c. In particular, we use jim_getopt_string(*goi, *buffer ,* len) for our tokenization needs. This function returns the full token, i.e. set buffer to “newtap\x00attack” and len to 13. Most of the time, we then proceed to interpret the token as standard string using standard string operators, which means openocd reads the token up to the NULL byte. This mean the string that we used to is “newtap”, not “newtap\x00attack”. In other wordswe read a different string from what is presented and potentially allowed NULL byte attack to happen. Subsequent work suggest that we can detect this easily, by comparing len with strlen(buffer). With the same example, len=13 but strlen(buffer) = 6. I tried a naïve method of fixing it, i.e. inserting this string length test into jim_get_opt_string: https://sourceforge.net/p/openocd/code/ci/master/tree/src/helper/jim-nvp.c#l208 to check string as below int jim_getopt_string(struct jim_getopt_info *goi, const char **puthere, int *len) { int r; Jim_Obj *o; const char *cp; r = jim_getopt_obj(goi, &o); if (r == JIM_OK) { cp = Jim_GetString(o, len); #### ADDED THIS IF If(len != strlen(cp)) { return JIM_ERR; } if (puthere) { *puthere = cp; } } return r; } My new code introduced an extra return state, so it is not surprising that openocd failed. In fact it segfault-ed with this simple configuration file: adapter driver <yourchoice> jtag newtap\x00attack newtap arm soc \ -irlen 4 -expected-id 0x4ba00477 Best regards Cinly
