replace fixed nodeList[512] with heap-allocated array

[uwezkhan]

fix(cgif_rgb): replace fixed nodeList[512] with heap-allocated array

crawl_decision_tree() used a 512-element stack array for breadth-first
tree traversal. A binary tree with colMax leaves requires exactly
2*colMax-1 nodes. With depthMax=8 this is 509 nodes (fits by margin),
but any increase to depthMax overflows the fixed array, corrupting the
call stack (CWE-121: Stack Buffer Overflow).

Replace with malloc((2 * colMax - 1) * sizeof(treeNode*)) so the
buffer is always exactly the right size regardless of colMax. Add a
NULL check on the allocation and free() after the loop.

Fixes: CWE-121 (Stack-based Buffer Overflow)

[Copilot]

Pull request overview

This pull request aims to fix a stack-based buffer overflow vulnerability (CWE-121) in the crawl_decision_tree() function by replacing a fixed-size 512-element stack array with a dynamically heap-allocated array sized exactly to the tree's needs.

Changes:

• Replaced fixed nodeList[512] stack array with heap-allocated malloc((2 * colMax - 1) * sizeof(treeNode*))
• Added NULL check for malloc failure with early return
• Added free(nodeList) at function exit
• Removed a line that appeared to contain a typo ("sawp" instead of "swap")

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The removal of this line appears to break the frequency array swap logic. The code is swapping elements at positions i and k in both pPalette and frequ arrays. For pPalette (lines 276-278), the swap is complete using a 3-element saveBlk buffer. For frequ, line 279 saves frequ[k] to saveNum, line 280 copies frequ[i] to frequ[k], but the removed line that would complete the swap (frequ[i] = saveNum) is now missing. This means the swap of frequency values is incomplete, which will corrupt the frequ array and lead to incorrect color quantization results. The removed line should be restored (with the typo "sawp" corrected to "swap" if desired).

[Copilot]

Silently returning on malloc failure leaves the decision tree in an incomplete state with no error indication to the caller. The function is void and has no mechanism to signal errors. The caller (create_decision_tree at line 324) has no way to detect that the tree construction failed, and will proceed to use an incomplete tree. Consider either: (1) changing the function signature to return an error code that the caller can check, or (2) if malloc failure is considered fatal and the application should terminate, add appropriate error handling such as calling exit() or another error handler.

[dloebl]

Hey @uwezkhan,
please refer to the contributing guidelines before opening any new pull requests or issues. Thanks.