Buffer overrun

[lorenzogatti]

When layout is right to left (-R) and input is long enough, with certain fonts (many in the "figletfonts40" JavE collection at http://www.jave.de/figlet/fonts.html and maybe others) the STRCAT call in addchar() writes past the end of the templine buffer.

Noticed on Windows 7, gcc 4.8.2, with both char and wchar_t variants.

Example test case:

figlet -f flowerpower.flf -R -x -S -p -w 80 abcdefghijklmnopqrstuvwxyz

I don't have time to prepare a patch at the moment, but I'll investigate further.

[lorenzogatti]

The problem is not initializing or reinitializing outputline[] buffer content to NUL, causing STRCAT to read more characters than it should because it doesn't find the normal NUL character when STRCAT doesn't start reading from the beginning of the buffer. There are also cases of corruption, with extra appended characters from uninitialized memory. Both problems fixed in clearline() (i.e. when the buffers are recycled) and myalloc() (first allocation).

[lorenzogatti]

Another case of buffer overrun in the same function, again for right to left layout: smushing away more characters that are contained in the outputline[] buffers, with STRCAT being passed an invalid pointer (past the end of an outputline[] buffer).

How is it possible to smush more characters than the length of the buffer? A single character can be wider than the current line, but smushamt() doesn't limit the amount of smushing to the length of the current line. Enormous amounts of smushing are possible with space-rich fonts, such as the Obanner collection.

Fixed in smushamt() by limiting the range of the result.

Test case:

$ figlet -f obanner132.flf -R -x -o -p -w 77 "Banner, o Banner"

[cmatsuoka]

Thanks, I'll check that and report back.

[lorenzogatti]

I created a pull request (#5) addressing both problems.

[jmccrohan]

Hi @cmatsuoka,

Any update on this?

Cheers, Jon

[cmatsuoka]

Oops, I think I forgot to provide a follow-up. Sorry, Lorenzo and Jon. Let me check exactly what happened and I'll post an update ASAP.

[cmatsuoka]

I'm checking the current code with valgrind and it seems that we have more memory issues. I didn't apply Lorenzo's code directly because it caused some of the regression tests to fail, so I'll investigate this further and rework the patch to cover all cases.

[cmatsuoka]

I changed Lorenzo's smush fix to be used only in the right-to-left case to prevent side effects in left-to-right smushing, and added a new regression test for the right-to-left memory corruption problem. I'm pushing this to master, please check if it works for you.