github/RRG-Proxmark3
1
0
Fork 0
mirror of https://github.com/RfidResearchGroup/proxmark3.git synced 2025-07-05 20:41:34 -07:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Fix a bunch of potential buffer overruns with CLIGetStrWithReturn :

Browse source 
Most prominent one was "lf pac clone --cn 123456789" causing garbage on the terminal
Some changed code was valid before but as ppl tend to copy-paste to new code,
better to uniformize and document usages of CLIGetStrWithReturn.

Exceptions left are when filling real buffers (bin, raw,...), not strings.
This commit is contained in:
Philippe Teuwen 2024-07-30 22:23:04 +02:00
parent 78510f33a0
commit 153064ddfa
16 changed files with 31 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

4
client/src/cmdanalyse.c
View file

@ -1025,8 +1025,8 @@ static int CmdAnalyseFoo(const char *Cmd) {
uint8_t data[256];
CLIGetHexWithReturn(ctx, 1, data, &datalen);
int data3len = 512;
uint8_t data3[512];
uint8_t data3[512] = {0};
int data3len = sizeof(data3) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated;
CLIGetStrWithReturn(ctx, 1, data3, &data3len);
CLIParserFree(ctx);

6
client/src/cmddata.c
View file

@ -3386,8 +3386,8 @@ static int CmdAtrLookup(const char *Cmd) {
arg_param_end
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int dlen = 128;
uint8_t data[128 + 1];
int dlen = sizeof(data) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, data, &dlen);
// bool selftest = arg_get_lit(ctx, 2);
@ -3574,12 +3574,12 @@ static int CmdBinaryMap(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int hlen = 5;
uint8_t hex[5 + 1];
int hlen = sizeof(hex) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, hex, &hlen);
int tlen = 40;
uint8_t template[40 + 1];
int tlen = sizeof(template) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 2, template, &tlen);
CLIParserFree(ctx);

3
client/src/cmdhficlass.c
View file

@ -4599,8 +4599,9 @@ static int CmdHFiClassEncode(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int bin_len = 63;
// TODO: very confusing sizes... buf of 70, parser len to 63 instead of 70-1, tests for len > 127, loop with 64...
uint8_t bin[70] = {0};
int bin_len = 63;
CLIGetStrWithReturn(ctx, 1, bin, &bin_len);
int key_nr = arg_get_int_def(ctx, 2, -1);

6
client/src/cmdhfjooki.c
View file

@ -442,8 +442,8 @@ static int CmdHF14AJookiDecode(const char *Cmd) {
arg_param_end
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int dlen = 16;
uint8_t b64[JOOKI_B64_LEN] = {0x00};
int dlen = sizeof(b64) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(b64, 0x0, sizeof(b64));
CLIGetStrWithReturn(ctx, 1, b64, &dlen);
bool verbose = arg_get_lit(ctx, 2);
@ -471,8 +471,8 @@ static int CmdHF14AJookiSim(const char *Cmd) {
arg_param_end
};
CLIExecWithReturn(ctx, Cmd, argtable, true);
int dlen = 16;
uint8_t b64[JOOKI_B64_LEN] = {0x00};
int dlen = sizeof(b64) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(b64, 0x0, sizeof(b64));
CLIGetStrWithReturn(ctx, 1, b64, &dlen);
CLIParserFree(ctx);
@ -611,8 +611,8 @@ static int CmdHF14AJookiClone(const char *Cmd) {
arg_param_end
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int blen = 16;
uint8_t b64[JOOKI_B64_LEN] = {0x00};
int blen = sizeof(b64) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(b64, 0x0, sizeof(b64));
CLIGetStrWithReturn(ctx, 1, b64, &blen);

2
client/src/cmdhfmf.c
View file

@ -9257,8 +9257,8 @@ static int CmdHFMFHidEncode(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
int bin_len = 120;
uint8_t bin[121] = {0};
int bin_len = sizeof(bin) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, bin, &bin_len);
wiegand_card_t card;

2
client/src/cmdhw.c
View file

@ -1358,8 +1358,8 @@ static int CmdConnect(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, true);
int p_len = FILE_PATH_SIZE;
char port[FILE_PATH_SIZE] = {0};
int p_len = sizeof(port) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated;
CLIGetStrWithReturn(ctx, 1, (uint8_t *)port, &p_len);
uint32_t baudrate = arg_get_u32_def(ctx, 2, USART_BAUD_RATE);
CLIParserFree(ctx);

4
client/src/cmdlf.c
View file

@ -272,12 +272,12 @@ int CmdLFCommandRead(const char *Cmd) {
CLIExecWithReturn(ctx, Cmd, argtable, false);
uint32_t delay = arg_get_u32_def(ctx, 1, 0);
int cmd_len = 128;
char cmd[128] = {0};
int cmd_len = sizeof(cmd) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 2, (uint8_t *)cmd, &cmd_len);
int extra_arg_len = 250;
char extra_arg[250] = {0};
int extra_arg_len = sizeof(extra_arg) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 3, (uint8_t *)extra_arg, &extra_arg_len);
uint16_t period_1 = arg_get_u32_def(ctx, 4, 0);

2
client/src/cmdlfem4x50.c
View file

@ -377,8 +377,8 @@ static int CmdEM4x50Brute(const char *Cmd) {
em4x50_data_t etd;
memset(&etd, 0, sizeof(etd));
int mode_len = 64;
char mode[64];
int mode_len = sizeof(mode) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, (uint8_t *) mode, &mode_len);
PrintAndLogEx(INFO, "Chosen mode: %s", mode);

1
client/src/cmdlfhid.c
View file

@ -375,6 +375,7 @@ static int CmdHIDClone(const char *Cmd) {
bool q5 = arg_get_lit(ctx, 7);
bool em = arg_get_lit(ctx, 8);
// TODO: very confusing sizes... buf of 70, parser len to 63 instead of 70-1, tests for len > 127, loop with 96...
int bin_len = 63;
uint8_t bin[70] = {0};
CLIGetStrWithReturn(ctx, 9, bin, &bin_len);

4
client/src/cmdlfkeri.c
View file

@ -244,8 +244,8 @@ static int CmdKeriClone(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
uint8_t keritype[2] = {'i'}; // default to internalid
int typeLen = sizeof(keritype);
uint8_t keritype[2] = {'i', 0}; // default to internalid
int typeLen = sizeof(keritype) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, keritype, &typeLen);
uint32_t fc = arg_get_int_def(ctx, 2, 0);

6
client/src/cmdlfpac.c
View file

@ -236,8 +236,8 @@ static int CmdPacClone(const char *Cmd) {
};
CLIExecWithReturn(ctx, Cmd, argtable, false);
uint8_t cnstr[9];
int cnlen = 9;
uint8_t cnstr[9] = {0};
int cnlen = sizeof(cnstr) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(cnstr, 0x00, sizeof(cnstr));
CLIGetStrWithReturn(ctx, 1, cnstr, &cnlen);
@ -329,7 +329,7 @@ static int CmdPacSim(const char *Cmd) {
CLIExecWithReturn(ctx, Cmd, argtable, false);
uint8_t cnstr[10];
int cnlen = 9;
int cnlen = sizeof(cnstr) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(cnstr, 0x00, sizeof(cnstr));
CLIGetStrWithReturn(ctx, 1, cnstr, &cnlen);

8
client/src/cmdlfpresco.c
View file

@ -189,8 +189,8 @@ static int CmdPrescoClone(const char *Cmd) {
uint8_t hex[4] = {0, 0, 0, 0};
CLIGetHexWithReturn(ctx, 1, hex, &hex_len);
uint8_t idstr[11];
int slen = 9;
uint8_t idstr[10];
int slen = sizeof(idstr) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(idstr, 0x00, sizeof(idstr));
CLIGetStrWithReturn(ctx, 2, idstr, &slen);
@ -292,8 +292,8 @@ static int CmdPrescoSim(const char *Cmd) {
uint8_t hex[4] = {0, 0, 0, 0};
CLIGetHexWithReturn(ctx, 1, hex, &hex_len);
uint8_t idstr[11];
int slen = 9;
uint8_t idstr[10] = {0};
int slen = sizeof(idstr) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
memset(idstr, 0x00, sizeof(idstr));
CLIGetStrWithReturn(ctx, 2, idstr, &slen);
CLIParserFree(ctx);

2
client/src/cmdlft55xx.c
View file

@ -1952,8 +1952,8 @@ static int CmdT55xxDangerousRaw(const char *Cmd) {
ng.bitlen = 0;
memset(ng.data, 0x00, sizeof(ng.data));
int bin_len = 127;
uint8_t bin[128] = {0};
int bin_len = sizeof(bin) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, bin, &bin_len);
ng.time = arg_get_int_def(ctx, 2, 0);

4
client/src/cmdsmartcard.c
View file

@ -1253,14 +1253,14 @@ static int CmdPCSC(const char *Cmd) {
CLIExecWithReturn(ctx, Cmd, argtable, true);
uint8_t host[100] = {0};
int hostLen = sizeof(host);
int hostLen = sizeof(host) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 1, host, &hostLen);
if (hostLen == 0) {
strcpy((char *) host, "localhost");
}
uint8_t port[6] = {0};
int portLen = sizeof(port);
int portLen = sizeof(port) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 2, port, &portLen);
if (portLen == 0) {
strcpy((char *) port, "35963");

2
client/src/emv/cmdemv.c
View file

@ -2041,7 +2041,7 @@ static int CmdEMVScan(const char *Cmd) {
uint8_t psenum = (channel == CC_CONTACT) ? 1 : 2;
uint8_t filename[FILE_PATH_SIZE] = {0};
int filenamelen = sizeof(filename);
int filenamelen = sizeof(filename) - 1; // CLIGetStrWithReturn does not guarantee string to be null-terminated
CLIGetStrWithReturn(ctx, 12, filename, &filenamelen);
CLIParserFree(ctx);

4
doc/cliparser.md
View file

@ -256,8 +256,8 @@ CLIGetHexWithReturn(\<context\>, \<opt index\>, \<store variable\>, \<ptr to sto
CLIGetStrWithReturn(\<context\>,\<opt index\>, \<uint8_t \*\>, \<int \*\>);
If failed to retrieve string, it will exit fct
uint8_t buffer[100];
int slen = sizeof(buffer); // <- slen MUST be the maximum number of characters that you want returned. e.g. Buffer Size
uint8_t buffer[100] = {0};
int slen = sizeof(buffer) - 1; // <- slen MUST be the maximum number of characters that you want returned. e.g. Buffer Size - 1 if you need it to be null-terminated!
CLIGetStrWithReturn(ctx, 1, buffer, &slen);
**string option**