Hi, I am not sure this is the proper place to make suggestions on CWE examples, but I have noticed a problem with CWE 129 - Example 3 (This is also a problem with CWE 125 - Example 1, and CWE 839 - Example 3). In each of the examples, the bad code has an insufficient check for out-of-bounds read, and the fix is to correct the check on out-of-bounds read. That part is fine, the issue, is when the insufficient check for the out-of-bounds read fails, the example code uses the invalid index to print out a value from the array, creating a new out-of-bounds read not covered by any check. The suggested fix does not cover this and given the description of the example, so I do not think this second out-of-bounds read was intentional.
One consequence of this issue is that if you are using the example code and the code after the fix to test code review techniques or static analysis, the code after the fix will still be flagged with an out-of-bounds read and that may confuse reviewers or developers of static analysis tools. With a very small change, I think this example can become more useful for these purposes. The current example is: int getValueFromArray(int *array, int len, int index) { int value; // check that the array index is less than the maximum // length of the array if (index < len) { // get the value at the specified index of the array value = array[index]; } // if array index is invalid then output error message // and return value indicating error else { printf("Value is: %d\n", array[index]); value = -1; } return value; } With a fix that changes only the line "if (index < len) {" to if (index >= 0 && index < len) { My suggestion is to have the example print out the invalid index instead of a value using the invalid index, so the changed code would be int getValueFromArray(int *array, int len, int index) { int value; // check that the array index is less than the maximum // length of the array if (index < len) { // get the value at the specified index of the array value = array[index]; } // if array index is invalid then output error message // and return value indicating error else { printf("Invalid index used: %d\n", index); value = -1; } return value; } The fix would remain: if (index >= 0 && index < len) { This change should apply to all CWE's using this example (I've noticed CWE-125, CWE-129 and CWE 839 using this example, but I would not be surprised if other instances are as well) Does my suggested change make sense? Again, please correct me if this is the wrong forum to suggest this change. With regards, John Thomas Field Application Engineer Technical Lead LDRA Technology