Failing String.to_integer/1 due to malicious requests

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:

• It’s not a security issue per se. However it might be, if the application relies on crashing process to succeed.
• Of course it is in any way a good choice to pattern match against the result of 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