When working on Phoenix projects I sometimes need to take integers as a string as user input from HTTP requests. When I use String.to_integer/1 to get the integer values, an attacker could easily craft a request that makes String.to_integer/1 raise an exception and thus crash the Erlang process.
Question: Is this failing String.to_integer/1 a security issue? Or does will only make the concerning Erlang process crash without further damage? What do I have to look after?
You might use guard clause to avoid this kind of error, no need to let it crash.
You might also use changeset to validate outside data, which is probably the cleanest way.
Another solution is to use Integer.parse, when there is a chance String.to_integer will fail.
Never trust user data 
Yes, of course, that is the cleanest solution. The question is, when I see such a potential crash in somewhere in production code, do I simply silently fix it and let it deploy one day or do I immediately shutdown the service and ship a hotfix?
I think You should prevent those errors.
But if You have them, it will just crash the process, I don’t think it’s urgent like patching Log4j
From the docs:
https://hexdocs.pm/elixir/String.html#to_integer/1
If you want to parse a string that may contain an ill-formatted integer, use
Integer.parse/1.
I feel your question is a bit philosophical and thus ignores context. There’s no right answer. The good question here is: is failing to parse the integer an actual error condition or is it acceptable?
But still, if we’re talking out of context then in my mind something like parsing an integer should never be allowed to error out. Pattern match the result of Integer.parse.
IMO, and in the specific context of web applications, an invalid input should NEVER make your process crash (thus resulting in a 500 error). Instead, it should always be handled by the application and trigger a 400 or 422 response.
Answering your direct question about security issues, then by itself it’s not a security issue. Phoenix will reply with a 500 error to the user which is not good UX, but assuming you’re in prod mode, it won’t leak any details. The memory of the process will be garbage collected and the system stability won’t be affected.
Now where it can be an issue is if your system relies on the process to do something security related and it crashes in the middle of that, leaving side effects or leaving something undone. Usually you’d use DB transactions to prevent issues from such matters but maybe you’re also talking to external systems, handling files, etc., where it’s not that simple. Only you can spot those dangerous places in your application.
Thanks for all the answers.
To wrap up:
Integer.parse/1
And I would add that, even though it’s not a security issue, it’s definitely a UX issue as @Nicd pointed out. As a user, you simply don’t want to see your application crash because you entered an invalid input. Instead, you want an error message that tells you what you did wrong.
The request doesn’t need to be malicious, it could simply be a mistake
I am thinking of cases where the exact request is formed in a LiveView by a phx-value-number={some.number} binding. So when I assume that the user does not explicitly inject a JSON record with a broken integer string, the issue will never be triggered and thus a “normal” user will never see the 500. Only the ones who manually inject some JSON data will see it, and the UX for those people is let’s say, secondary
Yes, in this case, you are or course correct. That input would definitely be “malicious”.
Semi-tangentially, this is an illustration of an aspect of software quality I call Robustness. Long story medium, I have a definition of software quality composed of six aspects, of which the third one is Robustness, with a short explanation that it should be hard to make the software malfunction or seem to. Looking for bad inputs and saying “naught user, no integer for you!” is one way to prevent both. Trapping (in Elixir, rescueing) the error is an alternative common in web apps, trapping things that would result in a fugly generic 500 page (maybe even a stack trace if your setup is particularly stupidly WRT security), and giving the user a much nicer notice like “We’re sorry, something went wrong, please try again.” This still seems a bit fragile, but nowhere near as “not only is it fragile we don’t even care” as a stack trace.
Further details at: Codosaurus: ACRUMEN
© Copyright Elixir Forum | Terms | Privacy & Cookies
