FT8 HEX message fix #27
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
October 11, 2021 00:00
This fixes problems with FT8 HEX messages, as described in etherkit#18
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 "<CA1LL" could do harm). In the case of "message data", size of input data should be checked before copy. And the case where I had problems: in my program, I have grid locator with more than bytes. I used the locator in other parts of my program, and then I passed it to wspr_encode(). wspr_encode() did a strcpy() fom my input data with len >> 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 etherkit#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; };
Bleach665
approved these changes
Nov 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes problems with FT8 HEX messages, as described in
#18