5 ms·

The message >NOTE: MAKE SURE THAT YOUR LINKS ENDS WITH .IMAGE_FORMAT AND DOESN'T HAVE ANY & CHARS AFTER BECAUSE THIS SYMBOL IS RESERVED BY GOLANG Makes me a b

The message >NOTE: MAKE SURE THAT YOUR LINKS ENDS WITH .IMAGE_FORMAT AND DOESN'T HAVE ANY & CHARS AFTER BECAUSE THIS SYMBOL IS RESERVED BY GOLANG Makes me a bit uncomfortable with it. Shouldn't input sanitization be one of the first things to consider when doing user-facing applications?

laumars 6y ago
The file names are ingested as a string so there is no way they’re being interpreted by the Go runtime (and in fact, Go being a AOT compiles language rather than a dynamic one means it wouldn’t be possible for file names to conflict like described in the readme). This suggests to me that the author is misunderstanding why ‘&’ causes POSIX shells to do weird things. I think what’s happening is the author tried a web URI that had an ampersand in, Bash then background the process and the author panicked thinking the Go runtime was parsing the ampersand rather than his or her $SHELL.
- bawolff 6y ago
  So in other words, OP's gut "Makes me a bit uncomfortable" is an understatement.
- guessmyname 6y ago
  > I think what’s happening is the author tried a web URI that had an ampersand in, Bash then background the process and the author panicked thinking the Go runtime was parsing the ampersand rather than his or her $SHELL. Yes, this is exactly what happened. The author says this is one of their first Go projects, which explains their misunderstanding. They still have a lot to learn, not only about Go but Bash as well. That being said, I am happy to see new programmers learn new things, especially in one of my favourite programming languages.
  - laumars 6y ago
    I remember a couple of years ago, one project also took file paths as a parameter and supported multiple parameters. In the readme the author stated that they hadn't yet written support for wildcards (globing) so each file needed to be included manually. There were surprised and pleased when I raised an "issue" pointing out that actually they get globing for free with their $SHELL. That experience really made me appreciate just how varied developers experiences were with the command line and also appreciative that people are still writing tools and happy to share them with the community even when they're not 40+ year old UNIX grey beards. It's good to see fresh folk adopt TUIs.
    - NotPavlovsDog 6y ago
      And we as a community in software should appreciate and celebrate when friendly and helpful support is provided to newcomers. Thank you!
      - irevenko 6y ago
        yes.
- kuroguro 6y ago
  No, I think the author doesn't understand how to urlencode parameters as the image URL is passed to the API like so: curl -s https://trace.moe/api/search?url=https://foobar/baz.jpg https://trace.moe/api/search?url=https://foobar/baz.jpg And in the project the URL is simply appended: https://github.com/irevenko/what-anime-cli/blob/main/cli/link_search.go#L39 https://github.com/irevenko/what-anime-cli/blob/main/cli/lin... The API provides both GET and POST methods of passing the image file or URL and the author has somehow mangled them both into one as well :) https://soruly.github.io/trace.moe/#/ https://soruly.github.io/trace.moe/#/
  - laumars 6y ago
    Good catch and that's definitely true as well but there's no way the author could have passed an ampersand into a Go string (ie as a parameter in said tool) without understanding the need to escape it in the first place. The shell would have parsed the ampersand as a reserved token out before the URI encoding bug was exposed. As first project though, it's a great learning exercise for the author. We all have to start somewhere.
mlex 6y ago
I don't get this concern. What specifically are you worried about here, that a bad parameter will crash the program? The code is there in case you wanted to verify it's not doing anything nefarious. This looks like just a cute script/program the author wanted to share with HN, I don't think it's a big deal if they didn't fully productionize the thing and just wanted it to be seen.
- kornbattery 6y ago
  The worry is that something in a link will cause the program to behave in unpredictable manner. At best it would crash, but it could do much more damage if given the chance. I don't know Golang, so I can't definitely confirm or deny any intricacies it may or may not have. Though as mentioned by others, it seems to essentially be a wrapper for a webpage
  - irevenko 6y ago
    Just do not use powershell l o l
    - laumars 6y ago
      It's reserved by POSIX shells as well. So either way you'd have to escape the ampersand.
- irevenko 6y ago
  Just do not use powershell l o l
- bawolff 6y ago
  It sounds like a shell injection vulnerability [i have not actually checked though]. Attack scenario is somewhat realistic ("friend" sends you an image in a manner that preserves filenames, you run command on file) Being blatently vulnerable is not the same thing as being not production ready.
jchw 6y ago
It’s a mistake. It’s certainly meant to say reserved by Powershell. Of course it will also not do the desired thing in POSIX shells, but in Powershell, rather than cut a URL short but likely keep it valid + fork, it will error out since & is reserved in Powershell. In reality what it should be telling you to do is shell escape the ampersand.
wccrawford 6y ago
Even if you sanitize inputs, you still have to sanitize outputs. And as the comments below show, the real problem here is that outputs weren't sanitized. Creating URLs or command line arguments without proper escaping is going to fail in so many ways, even on data that isn't malicious.

laumars 6y ago

The file names are ingested as a string so there is no way they’re being interpreted by the Go runtime (and in fact, Go being a AOT compiles language rather than a dynamic one means it wouldn’t be possible for file names to conflict like described in the readme). This suggests to me that the author is misunderstanding why ‘&’ causes POSIX shells to do weird things. I think what’s happening is the author tried a web URI that had an ampersand in, Bash then background the process and the author panicked thinking the Go runtime was parsing the ampersand rather than his or her $SHELL.

bawolff 6y ago
So in other words, OP's gut "Makes me a bit uncomfortable" is an understatement.
guessmyname 6y ago
> I think what’s happening is the author tried a web URI that had an ampersand in, Bash then background the process and the author panicked thinking the Go runtime was parsing the ampersand rather than his or her $SHELL. Yes, this is exactly what happened. The author says this is one of their first Go projects, which explains their misunderstanding. They still have a lot to learn, not only about Go but Bash as well. That being said, I am happy to see new programmers learn new things, especially in one of my favourite programming languages.
- laumars 6y ago
  I remember a couple of years ago, one project also took file paths as a parameter and supported multiple parameters. In the readme the author stated that they hadn't yet written support for wildcards (globing) so each file needed to be included manually. There were surprised and pleased when I raised an "issue" pointing out that actually they get globing for free with their $SHELL. That experience really made me appreciate just how varied developers experiences were with the command line and also appreciative that people are still writing tools and happy to share them with the community even when they're not 40+ year old UNIX grey beards. It's good to see fresh folk adopt TUIs.
  - NotPavlovsDog 6y ago
    And we as a community in software should appreciate and celebrate when friendly and helpful support is provided to newcomers. Thank you!
    - irevenko 6y ago
      yes.
kuroguro 6y ago
No, I think the author doesn't understand how to urlencode parameters as the image URL is passed to the API like so: curl -s https://trace.moe/api/search?url=https://foobar/baz.jpg https://trace.moe/api/search?url=https://foobar/baz.jpg And in the project the URL is simply appended: https://github.com/irevenko/what-anime-cli/blob/main/cli/link_search.go#L39 https://github.com/irevenko/what-anime-cli/blob/main/cli/lin... The API provides both GET and POST methods of passing the image file or URL and the author has somehow mangled them both into one as well :) https://soruly.github.io/trace.moe/#/ https://soruly.github.io/trace.moe/#/
- laumars 6y ago
  Good catch and that's definitely true as well but there's no way the author could have passed an ampersand into a Go string (ie as a parameter in said tool) without understanding the need to escape it in the first place. The shell would have parsed the ampersand as a reserved token out before the URI encoding bug was exposed. As first project though, it's a great learning exercise for the author. We all have to start somewhere.

mlex 6y ago

I don't get this concern. What specifically are you worried about here, that a bad parameter will crash the program? The code is there in case you wanted to verify it's not doing anything nefarious. This looks like just a cute script/program the author wanted to share with HN, I don't think it's a big deal if they didn't fully productionize the thing and just wanted it to be seen.

kornbattery 6y ago
The worry is that something in a link will cause the program to behave in unpredictable manner. At best it would crash, but it could do much more damage if given the chance. I don't know Golang, so I can't definitely confirm or deny any intricacies it may or may not have. Though as mentioned by others, it seems to essentially be a wrapper for a webpage
- irevenko 6y ago
  Just do not use powershell l o l
  - laumars 6y ago
    It's reserved by POSIX shells as well. So either way you'd have to escape the ampersand.
irevenko 6y ago
Just do not use powershell l o l
bawolff 6y ago
It sounds like a shell injection vulnerability [i have not actually checked though]. Attack scenario is somewhat realistic ("friend" sends you an image in a manner that preserves filenames, you run command on file) Being blatently vulnerable is not the same thing as being not production ready.

jchw 6y ago

It’s a mistake. It’s certainly meant to say reserved by Powershell. Of course it will also not do the desired thing in POSIX shells, but in Powershell, rather than cut a URL short but likely keep it valid + fork, it will error out since & is reserved in Powershell. In reality what it should be telling you to do is shell escape the ampersand.

wccrawford 6y ago

Even if you sanitize inputs, you still have to sanitize outputs. And as the comments below show, the real problem here is that outputs weren't sanitized. Creating URLs or command line arguments without proper escaping is going to fail in so many ways, even on data that isn't malicious.