From 101199e9f10eb253c502220a67f51804351cec2b Mon Sep 17 00:00:00 2001 From: Thomas Osterried Date: Mon, 11 Oct 2021 00:00:48 +0200 Subject: [PATCH 1/2] FT8 HEX message fix This fixes problems with FT8 HEX messages, as described in https://github.com/etherkit/JTEncode/issues/18 --- src/JTEncode.cpp | 49 ++++++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 29 deletions(-) diff --git a/src/JTEncode.cpp b/src/JTEncode.cpp index fcb7723..5a53e37 100644 --- a/src/JTEncode.cpp +++ b/src/JTEncode.cpp @@ -210,6 +210,8 @@ void JTEncode::wspr_encode(const char * call, const char * loc, const int8_t dbm // Ensure that the message text conforms to standards // -------------------------------------------------- + //:w + wspr_message_prep(call_, loc_, dbm_); // Bit packing @@ -1075,9 +1077,7 @@ void JTEncode::ft8_bit_packing(char* message, uint8_t* codeword) uint8_t n3 = 0; uint8_t qa[10]; uint8_t qb[10]; - char c18[19]; bool telem = false; - char temp_msg[19]; memset(qa, 0, 10); memset(qb, 0, 10); @@ -1088,10 +1088,13 @@ void JTEncode::ft8_bit_packing(char* message, uint8_t* codeword) // Has to be hex digits, can be no more than 18 for(i = 0; i < 19; ++i) { - if(message[i] == 0 || message[i] == ' ') + if(message[i] == 0) { break; } + // bugfix: "73 DL9SAU" was interpreted as hex message. Check further for non-hex-character + else if (message[i] == ' ') + continue; else if(hex2int(message[i]) == -1) { telem = false; @@ -1099,7 +1102,6 @@ void JTEncode::ft8_bit_packing(char* message, uint8_t* codeword) } else { - c18[i] = message[i]; telem = true; } } @@ -1107,32 +1109,21 @@ void JTEncode::ft8_bit_packing(char* message, uint8_t* codeword) // If telemetry if(telem) { - // Get the first 18 hex digits - for(i = 0; i < strlen(message); ++i) - { - i0 = i; - if(message[i] == ' ') - { - --i0; - break; - } + char *p; + + // skip leading blanks + for (p = message; *p == ' '; p++) ; + { char buf[19]; + char *q = buf; + // Copy until end or first ' ' ("AA BB" is invalid), + // and convert all chars to uppercase. + while (*p && *p != ' ') + *q++ = toupper(*p++); + *q = 0; + // right-align + sprintf(message, "%18.18s", buf); } - memset(c18, 0, 19); - memmove(c18, message, i0 + 1); - snprintf(temp_msg, 19, "%*s", 18, c18); - - // Convert all chars to uppercase - for(i = 0; i < strlen(temp_msg); i++) - { - if(islower(temp_msg[i])) - { - temp_msg[i] = toupper(temp_msg[i]); - } - } - strcpy(message, temp_msg); - - uint8_t temp_int; temp_int = message[0] == ' ' ? 0 : hex2int(message[0]); for(i = 1; i < 4; ++i) @@ -1645,4 +1636,4 @@ void JTEncode::pad_callsign(char * call) // { // // return 1; // } -} \ No newline at end of file +} From 82a48f5057d9bdb982fc470cb4d764b6c3ab3424 Mon Sep 17 00:00:00 2001 From: Thomas Osterried Date: Mon, 11 Oct 2021 02:11:31 +0200 Subject: [PATCH 2/2] Suggested fixes 1. strcpy issues fixed: It's betterr to not trust user input. For some input types, like wspr type2 message callsign, trust may be ok (but even here, invalid input like "DL///CA1LL" or "> 6 bytes to a local variable with size 7. => board rebooted 2. latlon_to_grid function grid result Grid is a string of 6 bytes plus \0. Documentation of the latlon_to_grid function says, user has to provide an array of min 7 bytes. strncpy(ret_grid, grid, 6) copied 6 bytes. grid is 6 bytes. strncpy does not add \0 if length is reached (which is the case here). -> 7 instead off 6 is the fix. Or: just strcpy(). Because both functions cannot assure that the buffer the user provided was correct. 3. char callsign with size 12 should better be 13. See https://github.com/etherkit/JTEncode/issues/28 4. Just a question: In ft_message_prep(), we have char temp_msg[14]; snprintf(temp_msg, 14, "%13s", message); printf "%13s" right-aligns (-> " foo"). Did you mean "%.13s" (fored cut at position 13)? Documentation: diff --git a/src/JTEncode.cpp b/src/JTEncode.cpp index 5a53e37..4f87006 100644 --- a/src/JTEncode.cpp +++ b/src/JTEncode.cpp @@ -67,7 +67,7 @@ void JTEncode::jt65_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); !!!!! ^ strncpy copies 13 bytes. If message is shorter 13 bytes, rest is filled with \0. !!!!! But if souorce msg is >= 13 bytes, strncpy does not null-terminate. !!!!! memset assured, that message[13] is 0. -> We are still null-terminated after this operation // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -111,7 +111,7 @@ void JTEncode::jt9_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -160,7 +160,7 @@ void JTEncode::jt4_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -205,8 +205,8 @@ void JTEncode::wspr_encode(const char * call, const char * loc, const int8_t dbm char call_[13]; char loc_[7]; uint8_t dbm_ = dbm; - strcpy(call_, call); - strcpy(loc_, loc); + snprintf(call_, 13, "%.12s", call); + snprintf(loc_, 7, "%.6s", loc); !!!!!! In this case, you did not memset local variables call_ and loc_. !!!!!! Thus, strncpy(loc_,..., 7) would not terminate if user input is more than 6 bytes. !!!!!! loc_[6] = 0; is one solution. The other ohne is snprintf, here with enforced knoowlege of variable size (7), !!!!!! and with left aligned and cut "%.6s" loc. // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -429,7 +429,7 @@ void JTEncode::ft8_encode(const char * msg, uint8_t * symbols) char message[19]; memset(message, 0, 19); - strcpy(message, msg); + strncpy(message, msg, 18); // Bit packing // ----------- @@ -498,7 +498,7 @@ void JTEncode::latlon_to_grid(float lat, float lon, char* ret_grid) grid[4] = (char)((uint8_t)(lon * 12) + 'a'); grid[5] = (char)((uint8_t)(lat * 24) + 'a'); - strncpy(ret_grid, grid, 6); + strncpy(ret_grid, grid, 7); } /* Private Class Members */ @@ -689,7 +689,7 @@ void JTEncode::wspr_message_prep(char * call, char * loc, int8_t dbm) } call[12] = 0; - strncpy(callsign, call, 12); + strncpy(callsign, call, 13); !!!!! call with length of 12 is a loocal variable. In this part oof the code, call may contain a null-terminated !!!!! string with 12 bytes plus 0. It's copied to global variable callsign with size of 12. !!!!! strncpy() works wel with size 12. But callsign is not null-terminateed. It may not need to. But in the following !!!!! code, tests like strchr(callsign, '...') are made - and these will run out beyond the array. !!!!! I see no harm in enlargeing the size of callsign fromo 12 to 13. // Grid locator validation if(strlen(loc) == 4 || strlen(loc) == 6) diff --git a/src/JTEncode.h b/src/JTEncode.h index 04718a5..9ab45da 100644 --- a/src/JTEncode.h +++ b/src/JTEncode.h @@ -258,7 +258,7 @@ private: uint8_t crc8(const char *); void pad_callsign(char *); void * rs_inst; - char callsign[12]; + char callsign[13]; char locator[7]; int8_t power; }; --- src/JTEncode.cpp | 16 ++++++++-------- src/JTEncode.h | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/JTEncode.cpp b/src/JTEncode.cpp index 5a53e37..4f87006 100644 --- a/src/JTEncode.cpp +++ b/src/JTEncode.cpp @@ -67,7 +67,7 @@ void JTEncode::jt65_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -111,7 +111,7 @@ void JTEncode::jt9_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -160,7 +160,7 @@ void JTEncode::jt4_encode(const char * msg, uint8_t * symbols) { char message[14]; memset(message, 0, 14); - strcpy(message, msg); + strncpy(message, msg, 13); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -205,8 +205,8 @@ void JTEncode::wspr_encode(const char * call, const char * loc, const int8_t dbm char call_[13]; char loc_[7]; uint8_t dbm_ = dbm; - strcpy(call_, call); - strcpy(loc_, loc); + snprintf(call_, 13, "%.12s", call); + snprintf(loc_, 7, "%.6s", loc); // Ensure that the message text conforms to standards // -------------------------------------------------- @@ -429,7 +429,7 @@ void JTEncode::ft8_encode(const char * msg, uint8_t * symbols) char message[19]; memset(message, 0, 19); - strcpy(message, msg); + strncpy(message, msg, 18); // Bit packing // ----------- @@ -498,7 +498,7 @@ void JTEncode::latlon_to_grid(float lat, float lon, char* ret_grid) grid[4] = (char)((uint8_t)(lon * 12) + 'a'); grid[5] = (char)((uint8_t)(lat * 24) + 'a'); - strncpy(ret_grid, grid, 6); + strncpy(ret_grid, grid, 7); } /* Private Class Members */ @@ -689,7 +689,7 @@ void JTEncode::wspr_message_prep(char * call, char * loc, int8_t dbm) } call[12] = 0; - strncpy(callsign, call, 12); + strncpy(callsign, call, 13); // Grid locator validation if(strlen(loc) == 4 || strlen(loc) == 6) diff --git a/src/JTEncode.h b/src/JTEncode.h index 04718a5..9ab45da 100644 --- a/src/JTEncode.h +++ b/src/JTEncode.h @@ -258,7 +258,7 @@ class JTEncode uint8_t crc8(const char *); void pad_callsign(char *); void * rs_inst; - char callsign[12]; + char callsign[13]; char locator[7]; int8_t power; };