5 ms·

Just a heads-up, you might want to take down the example website. Now that it's been posted to HN you might see malicious actors. I also think it's prone to SQ

• lupire 5y ago
  Why? That shows important information about the quality of the product.
• grey-area 5y ago
  Yes, don't do this: https://github.com/rehacktive/caffeine/blob/master/database/postgres_database.go https://github.com/rehacktive/caffeine/blob/master/database/... "INSERT INTO %v (id, data) VALUES('%v','%v') ON CONFLICT (id) DO UPDATE SET data = '%v'" Use prepared statements and parameters passed to the db driver, not building strings with strings or you are vulnerable to sqli. I'd also avoid using %v anyway when building strings - safer to use a specific type like %d for int.
  • johnday 5y ago
    You're right of course, but I think the idea of this software is that the user (and, by extension, their input) are trusted.
    • grey-area 5y ago
      Good to start with the right habits. People will put this on the internet, the author did for example.
    • throw_m239339 5y ago
      > You're right of course, but I think the idea of this software is that the user (and, by extension, their input) are trusted. No, you should enforce the most basic security practices even if the users are "trusted". Somebody might put that on the internet for a demo for client and get hacked in no time, because the code is opened to SQL injection. This isn't acceptable. There are minimum security standards every project should follow. And since none of the minimum security standards are implemented in that project, I would not recommend using it until they are.
      • johnday 5y ago
        I'm not sure what you are "no"-ing. I agreed with the parent comment.
  • aw4y 5y ago
    the reason I went "quick'n dirty" is for the prototyping nature of the project. But I'll fix this anyway, thanks!
    • robmccoll 5y ago
      Using the db Exec / Query / Query row is the same amount of code and effort the fmt.Sprintf statements. Even in quick-and-dirty mock-ups, it's a good idea to not cut corners there.
    • Cthulhu_ 5y ago
      There's nothing as permanent as a temporary solution. There's been countless SQL injection vulnerabilities exploited over the past decades with the "I'll fix it later" mindset. Start with prepared statements by default, they are not more work than formatting strings.
  • mahogany 5y ago
    Are you saying that %v should be avoided in general, or just in the context of queries? (Go noob who doesn't understand %v)
    • robmccoll 5y ago
      In queries, you should use the database/sql.DB interface if possible with your database https://pkg.go.dev/database/sql#DB.Exec https://pkg.go.dev/database/sql#DB.Exec It should sanitize / quote arguments for you and protect against SQL injection. Note that this doesn't mean all data sanitization is performed, just the basic '; do my stuff here; -- type of things.
    • grey-area 5y ago
      First, don't build strings like this for sql. But for other strings built with sprintf, yes I am saying don't use %v unless for debugging, it's just one more avenue where you might be surprised by input, even if you're not building strings for sql. For example, someone might do this with user supplied data: myoutput := fmt.Sprintf("user:%v",userID) and if userID is a string like "foo" it'll end up in their string and they won't get what they expect. So better to just put another guardrail on there and insist that the param is an integer or whatever you expect by using %d which will only accept an int - this means you have to convert it to an integer first. Many vulnerabilities are caused by data not being in the format people expect (not just sqli).
      • mahogany 5y ago
        Thanks for the explanation! That is a good point about being data-aware.
• samstave 5y ago
  >>Now that it's been posted to HN you might see malicious actors. This needs to be an HN fN Masterclass by @Dang.... We should have regular visibility into /how/ HN is used by malicious actors.... HN has enough pedigree to get some insight from some of the top security folks... and it would be REALLY good to know how HN is being harvested in this space.