[Security] lua used in this project is vulnerable

[the-Chain-Warden-thresh]

CVE-2020-24370 is a security vulnerability in lua. Although the CVE decription in NVD said that this CVE only affected lua 5.4.0, according to lua this CVE actually existed since lua 5.2. The root cause of this CVE is the negation overflow that occurs when you try to take the negative of 0x80000000. Thus, this CVE also exists in this project. The file which contains vulnerable functions is lua/src/ldebug.c.
You can easily fix this vulnerability by referring to this patch.

[chrisant996]

Thanks for the notification.

You can easily fix this vulnerability by referring to this patch.

No, the "easily" part is misleading, and I think it's based on an assumption without having looked at the 5.2 sources or the history between 5.2 and 5.4.

The vararg implementation in Lua was rewritten twice after 5.2.4 and before the commit that fixes the CVE:

• First rewrite: lua/lua@5c8770f
• Second rewrite: lua/lua@b137993

The cited patch relies on both of the rewrites (which look intertwined with other changes over time, as well).

[chrisant996]

The issue is clearly not urgent, and I'll see about coming up with a fix over the next week or so.

[chrisant996]

Also, to be clear: Clink does not perform the vulnerable operation.

Malicious third party scripts might perform the vulnerable operation. But if you have malicious third party scripts running, then this vulnerability is pretty uninteresting since there are so many much worse things that malicious scripts can do.

In fact, I don't see any reason to even waste time fixing the CVE. Clink does not perform the vulnerable operation, so Clink by itself is not vulnerable.

The only way to become vulnerable would be to run a malicious script. And if a malicious script is somehow able to get run, then there's an infinite number of worse and more impactful things it can do simply by using Lua itself without exploiting any bugs.

This isn't even worth addressing as "defense in depth". Both "Security" and "Moderate" are misleading tags in this case.

[the-Chain-Warden-thresh]

Thanks for the notification.

You can easily fix this vulnerability by referring to this patch.

No, the "easily" part is misleading, and I think it's based on an assumption without having looked at the 5.2 sources or the history between 5.2 and 5.4.

The vararg implementation in Lua was rewritten twice after 5.2.4 and before the commit that fixes the CVE:

• First rewrite: lua/lua@5c8770f
• Second rewrite: lua/lua@b137993

The cited patch relies on both of the rewrites (which look intertwined with other changes over time, as well).

Sorry for the late reply. I'm busy with searching for repos who have the same problem and try to fix them. One of my PR have been merged and I know quite well about this CVE right now.
Though this CVE barely affects clink, a fix for the potential negation overflow does no harm. If it's too busy for you to handle this, I'm glad to open a PR to fix it.

[chrisant996]

For me the issue is the fix isn't a straight port, so I want to ensure the code is well understood (including by myself) and the fix is verifiable. Both that it solves the issue, and doesn't introduce regressions.

I agree there's no harm in fixing the issue even if it isn't a credible threat. But there would be harm in an incorrect fix, and so I don't want to accept a fix unless it either comes from the Lua org or is well understood by myself (because I'm responsible for the effects).

I partially understand the change, but not fully yet. I can't accept a PR until I sufficiently understand the surrounding code and the change.

[goodusername123]

@chrisant996
I have some good news! The process of bringing this fix into Lua 5.2.4 based on a official source is a lot more straightforward then originally thought as Lua 5.3.6 (the very last release of Lua 5.3 which was also released in the same month as 5.4.1) actually includes a fix for this bug and funnily enough the implementation exactly matches the pull request linked to in this comment.

I ended up finding this out while comparing dates and doing diffs on releases of Lua, since minor releases/versions of Lua seem to be quite poorly documented as I'm not sure if changelogs even exist at all.

So in conclusion for a official version of a fix for this bug that's way easier to properly translate into Lua 5.2.4 simply do a diff of ldebug.c between Lua 5.3.5 and Lua 5.3.6 or simply look at libretro/RetroArch#16190 since it ends up being identical,
And also the only difference between the area of code in Lua 5.2 and Lua 5.3 where the fix targets is cast_int which was introduced in this commit during Lua 5.3's development: lua/lua@e723c75

Oh and also as a final extra note I guess the Lua 5.3 version of this fix doesn't show up in the Lua GitHub mirror since it only tracks the development branch the Lua 5.3 version of this fix appears in the Lua GitHub mirror under the v5.3 branch here (I just found this out): lua/lua@b5bc898

[chrisant996]

@goodusername123 ah that's great, thanks! I'll take a look tonight and then merge it.