4

I need to take input from the user of a simple bit of math, they want to solve. Rather than parse there input myself, I want to just use the lua, loadstring function to load their input and evaluate it.

But I don't want to let them execute abaritary code, So I am passing the string though a whitelist to make sure they are only using mathematical characters.

function calc(inputEquationStr)
    local startValid, endValid = string.find(inputEquationStr,"[%(%)%.%^%*%+%-/%d]+")
    if startValid>1 or endValid<string.len(inputEquationStr) then
        print("Error: Input doesn't look like math.")
    else
        local funcStr = "return " .. inputEquation 
        local mathFunction = loadstring(funcStr)
        assert(mathFunction, "Syntax Error, in input")

        print(mathFunction())
    end

end

function calcOuter(inputStr)
    local success, err = pcall(calc, inputStr)
    if not success then
        print("Error : " .. tostring(err))
    end

end

So the important part as i see it is that I whitelist to only allow +,-,.,*,/,^, () and digits. Which is done with string.find(inputEquationStr,"[%(%)%.%^%*%+%-/%d]+"

Is this going to be enough to protect against injection? Is there some kind of way, like hex-encoded strings that I need to watch out for?

Anders
  • 64,406
  • 24
  • 178
  • 215
Frames Catherine White
  • 289
  • 2
  • 11
  • It looks safe, but I don't know about Lua; solving this problem in code is [really easy](https://en.wikipedia.org/wiki/Reverse_Polish_notation#Postfix_algorithm) though – copy Aug 22 '13 at 05:47
  • For prefix notation sure. or if I had a decent parsing library sure. But the particular lua variant i am using isn't compatible with libraries written in C (or anything else other than lua). So I don't have a parser – Frames Catherine White Aug 22 '13 at 06:42
  • @Oxinabox: why don't you use separate Lua states for separate purposes? You could for example make it impossible to load any external libs in one "sandbox" Lua state and use another one for the usual Lua business. – 0xC0000022L Aug 22 '13 at 15:32
  • 0xC000: I had never heard of them until now. I will look into http://stackoverflow.com/questions/4201284/what-is-a-lua-state – Frames Catherine White Aug 22 '13 at 15:34

3 Answers3

2

I would recommend spending a little effort writing a parser for the arithmetic expressions in the input rather than dynamically executing it as Lua code. Even if you haven't got a third party parsing library available, it's not too difficult to write a lexer that splits up a string in relevant symbols, and then traversing those with a state-machine or recursive function.

Remote-code injection vulnerabilities are very serious, and there are so many possibile avenues for attacks here: what happens if an attacker tries to divide by zero? Or tries exploiting a bug in your Lua interpreter? Or does something weird with character encodings? Or uses some obscure language feature you did not think about? Considering all these matters is probably a lot more work than writing a parser for simple arithmetic expressions.

Even if you were able to choose a whitelist that would render this completely secure, it might still be possible that some future version of Lua introduces some new syntax or feature that would allow a new attack on this.

Another potential issue: if you would want to introduce some new feature in the future (e.g. also allowing users to compute square roots) you would either need to drop this approach and start building a parser anyway; or expand your whitelist, making it more complicated and therefore more likely to be insecure.

AardvarkSoup
  • 577
  • 2
  • 10
1

I did a similar thing a while ago and my solution was that I set the environment to the math functions and that is all there is. If any other function is called, it will error with attempt to index nil.

Creator
  • 134
  • 2
  • 2
-2

Hmm OK, I assume you are using Lua 5.1 (loadstring) but loadstring also excepts bytecode and bytecode is numbers. BOOM OK let me also assume you are not worried by the user doing anything else such as causing the process to hang and you are using an unbreakable Lua sandbox; how are you protecting the binary? It is very easy to bypass your check or change any C/Lua code by injecting and patching that.

Cuts to the chase: Don't bother trying to prevent injection unless you want to spend a considerable amount of time playing cat and mouse with a cracker, a cracker that generally has more time you do. Do spend most of your time preventing injection if this is a real concern which would cost you more money than just leaving or it is mission critical.

Jim
  • 1