Time-of-check vs time-of-use "bug" in src/string_utils.c

[kokke]

This PR fixes a small issue where pointers are checked against NULL but the check happens after the pointer was already dereferenced.

Specifically in src/string_utils.c line 351:

strlen(ptr) dereferences 'ptr' so if 'patt', 'repl' or 'string' was ever NULL, the program would crash before reaching the assert-statement.

See below lines 342, 343, 344 + 351 for comments explaining the issue.

337   char *
338   search_and_replace(char *patt, const char *repl, const char *string, int cursorpos,
339                      int *line, int *col)
340   {
341     int i, j, k;
342     int pattlen = strlen(patt);
                      ^^^^^^^^^^^^ pointer 'patt' gets dereferenced here
343     int replen = strlen(repl);
                     ^^^^^^^^^^^^ pointer 'repl' gets dereferenced here
344     int stringlen = strlen(string);
                     ^^^^^^^^^^^^ pointer 'string' gets dereferenced here
345     int cursor_found = FALSE;
346     int current_line = 1;
347     int current_column = 0;
348     size_t scratchsize;
349     char *scratchpad, *result;
350   
351     assert(patt && repl && string);
               ^^^^ checks pointers != NULL, but they were already dereferenced above

My suggested solution is to move the assert-statement up to the top of the function, before the pointers are used (passed to strlen()) - so the code becomes like this:

337  char *
338  search_and_replace(char *patt, const char *repl, const char *string, int cursorpos,
339                     int *line, int *col)
340  {
341    assert(patt && repl && string); /* check pointers before using them */
342  
343    int i, j, k;
344    int pattlen = strlen(patt);
345    int replen = strlen(repl);
346    int stringlen = strlen(string);

[hanslub42]

Thanks for the bug report and patch!!! I implemented your proposal a bit differently as I still try to follow the C89 "no statements before declarations" rule (which is probably completely obsolete by now, but still, rlwrap promises to be especially useful on obsolete systems....

See b3c0890affc8732b2238acf93c2b63fd5bb0677b

[kokke]

Hi @hanslub42 - that totally makes sense to me 👍

Thanks for considering my inputs - I hope you have a nice day 😃