bug(developer): incorrect handling of second parameter of index in kmcmplib compiler

[markcsinclair]

Describe the bug

If GetXStringImpl() in Compiler.cpp is called with an index statement with either a non-numeric second parameter or a missing second parameter no error message is generated.

With a valid store b, the statement index(b,g) will be interpreted as equivalent to index(b,0)

The fix needs to check that r is a number before applying atoiW(r) in the index section of GetXStringImpl() (ln 2022).

With a valid store b, the statement index(b,) will result in the compiler searching through memory for non-null text, which is then interpreted as the second parameter (usually equivalent to 0). There is an additional risk of heap corruption due to the pointer having overrun the parameter array.

The problem arises in the u16tok() function called on ln 2016 of the index section of GetXStringImpl(). In ln 262 of kmx_u16.cpp, the code uses u16chr() to step past the second and subsequent characters of the delimiter, but at this point q==0, and the loop runs until a non-null, non-delimiter character is found. The loop needs to check that q is not null. In addition, a check is needed that the first character of r (the return from strtok()) is not null in ln 2017 of the index section of GetXStringImpl().

Expected behavior

It is probably sufficent to raise CERR_InvalidIndex in both cases. but there may be an argument for a new error.

Keyman apps

• Keyman for Android
• Keyman for iPhone and iPad
• Keyman for Linux
• Keyman for macOS
• Keyman for Windows
• Keyman Developer
• KeymanWeb
• Other - give details at bottom of form

[markcsinclair]

I have developed a fix for the issue: fix(developer): handle second parameter of index correctly in kmcmplib compiler #11815

[markcsinclair]

The decision is to break this out into (at least) two PRs. #11815 will tackle the non-integer offset, and another one will address the use of u16chr() with *q==0 in u16tok().

[markcsinclair]

The issue needs to stay open until both bugs are fixed.