4 ms·

I find it odd there's no user ID instead of the username. But what I find even more concerning is it's not sanitizing the username to protect against SQL inject

I find it odd there's no user ID instead of the username. But what I find even more concerning is it's not sanitizing the username to protect against SQL injection.

debaserab2 7y ago
How can you know that there's no sanitization just from this blog post?
- MichaelApproved 7y ago
  I think OP is getting the wrong idea from the screen shot in the article. There's a pic of an SQL query being run from a terminal. They might be thinking that it's a screen shot from the actual code.
  - aloknnikhil 7y ago
    It's open source. https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se...
    - anyfoo 7y ago
      This looks perfectly fine? Are you referring to the fact that they don't escape special characters like ' in the string? With the proper interfaces, like using prepared statements (which this likely uses in the backend, if this is an actual SQL db), instead of putting queries together just by concatenating strings, escaping strings is not only unnecessary but actively harmful.
- deleted 7y ago
  [deleted]
- aloknnikhil 7y ago
  https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se... It's all open source
  - dwohnitmok 7y ago
    It looks properly sanitized to me. The DB library being used is creating prepared SQL statements underneath the hood.
voidmain0001 7y ago
Agreed. The username is simply a human friendly representation of an internal unique ID use in the database. What if the user decides to change their name from "CPU" to "CPUZ"? They would lose all of their experience. I really the implementation is not as shown in the screen shot.
- aloknnikhil 7y ago
  It most definitely is https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se... Unless, I read that wrong
  - recursive 7y ago
    The screenshot is a SQL query. There's no SQL in the linked haskell source. I'm not sure what you're getting at.
  - kccqzy 7y ago
    The code you linked looks good to me. The Beam library handles this properly. Where you do think the injection issue is?
dwohnitmok 7y ago
The code is sanitizing the username by running a prepared SQL statement. It's not doing SQL string concatenation.
tummybug 7y ago
Genuine question, if usernames are unique what is the problem with using them as the primary key of the table instead of an arbitrary value from a sequence, you also get the bonus of not needing a unique constraint on the username column.

I find it odd there's no user ID instead of the username. But what I find even more concerning is it's not sanitizing the username to protect against SQL injection.

debaserab2 7y ago

How can you know that there's no sanitization just from this blog post?

MichaelApproved 7y ago
I think OP is getting the wrong idea from the screen shot in the article. There's a pic of an SQL query being run from a terminal. They might be thinking that it's a screen shot from the actual code.
- aloknnikhil 7y ago
  It's open source. https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se...
  - anyfoo 7y ago
    This looks perfectly fine? Are you referring to the fact that they don't escape special characters like ' in the string? With the proper interfaces, like using prepared statements (which this likely uses in the backend, if this is an actual SQL db), instead of putting queries together just by concatenating strings, escaping strings is not only unnecessary but actively harmful.
deleted 7y ago
[deleted]
aloknnikhil 7y ago
https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se... It's all open source
- dwohnitmok 7y ago
  It looks properly sanitized to me. The DB library being used is creating prepared SQL statements underneath the hood.

voidmain0001 7y ago

Agreed. The username is simply a human friendly representation of an internal unique ID use in the database. What if the user decides to change their name from "CPU" to "CPUZ"? They would lose all of their experience. I really the implementation is not as shown in the screen shot.

aloknnikhil 7y ago
It most definitely is https://github.com/RoganMurley/Ring-of-Worlds/blob/master/server/src/User.hs https://github.com/RoganMurley/Ring-of-Worlds/blob/master/se... Unless, I read that wrong
- recursive 7y ago
  The screenshot is a SQL query. There's no SQL in the linked haskell source. I'm not sure what you're getting at.
- kccqzy 7y ago
  The code you linked looks good to me. The Beam library handles this properly. Where you do think the injection issue is?

dwohnitmok 7y ago

The code is sanitizing the username by running a prepared SQL statement. It's not doing SQL string concatenation.

tummybug 7y ago

Genuine question, if usernames are unique what is the problem with using them as the primary key of the table instead of an arbitrary value from a sequence, you also get the bonus of not needing a unique constraint on the username column.