custom_merge.lua SQL injection vulnerability

Hi everybody, I hope you had (or are having) a nice holiday.

Does anybody use custom_merge.lua?
I think I have found an SQL injection vulnerability in the code. See lines 64-67:

      if tostring(Node):match('0x') then
         Buffer:write(Node)
      else
         Buffer:write(Conn:quote(tostring(Node)))

I.e. if the string contains the substring 0x anywhere, it is pasted unquoted into the SQL command. This leads to an error if inserting a string like image640x480.jpg, and can wreak havoc if you try to insert a string like ‘0x’); DROP TABLE VeryImportantTable; — or something (you get the general idea).

I propose you might want to change it to something like:

      if tostring(Node):match('^0x%x+$') then
         Buffer:write(Node)
      else
         Buffer:write(Conn:quote(tostring(Node)))

So, to be pasted without quotation, the string must consist of 0x, and then hex digits only.

Hi Robin – I took time off over the holidays and meant to reply earlier. Good catch.

Maybe a good way to share this would be if we were to put the custom_merge.lua code into our GIT repository then you could fork the GIT repo and we could pull the changes back into our main repo. Would you be up for that? It might provide a way for you to share some of the other pieces you have been doing.

There is a lot of work being done on the source code for Iguana 6 incidentally which ought to make it possible to support having Iguana put source code it at least a global fossil repository – if not a GIT repository.

Hi Eliot,

sure, if you put custom_merge into the GIT repo, I can add my little fancy doings. Definitely better than spreading them all over various forum discussion threads!

I’m curious to see Iguana 6, the new source repository might be just the thing for us, for our next version.

Kind regards

Robin

I got Wade to add the module to the repo now:

https://github.com/interfaceware/iguana-web-apps/blob/master/shared/custom_merge.lua

So you should be in position to go and fork it now. I’d suggest only putting minor bug fixes into the vanilla version and then having your own more customized one separate.

I think Wade will put in the fix in shortly.

Okay the patch is in there:

https://github.com/interfaceware/iguana-web-apps/commit/43ff3e6c0f28934ad50101acec66cbaf6f3b8f60

See this thread for what’s happening with version control:

http://help.interfaceware.com/forums/topic/sneak-peak-of-improved-version-control-management-for-iguana-6