Bug #6385
closedmalformed IMSI
0%
Description
Hello,
I a pySim-shell, If we edit an IMSI on a SIM card and we miswrite it, we couldn't not read it or modify it anymore (see picture of the bug)
In the attached file, there is a patch to handle this
Files
Updated by laforge about 2 months ago
- Status changed from New to Rejected
thanks for the pach.
I'm not sure how tolerant we need to be when it comes to parsing invalid data.
The user can always use pySim-shell `update_binary` to write either some valid data, or simply set everything to ff.
Also, pySim-shell `update_binary_decpded` should also work, as it doesn't try to decode the previous data.
I think the only operation affected is `edit_binary_decoded` - and here I'm not sure we are able to adjust our code for hundreds of different EF decoders to always support whatever malformed data may be present in a file.
Updated by n4n5 about 2 months ago
The patch would be very useful for newbies (like me)
Here is two use case:
- First: you miswrite your IMSI, you can't read it or write it anymore
-> with the patch, you can !
- Second: you get the card from someone, but he purposely miswrite the IMSI
-> with the patch you can handle that by writing a new one
Btw the patch is really tiny (just a try except)
Updated by laforge about 2 months ago
On Sat, Mar 09, 2024 at 09:45:22PM +0000, n4n5 wrote:
The patch would be very useful for newbies (like me)
Possibly. However, hundreds of similar patches would be needed for all the hundreds
of different file decoders, and then we would have to add test coverage to test all of that.
It is not manageable. There are almost no people contributing to pySim, and I cannot add
this additinal burden.
As I stated, `update_binary_decoded` and `update_binary` will just work fine to recover from
any file with invalid contents. This will always work, on any file.
I would happily accept a patch for the pySim-shell user manual which explains this to users.
Updated by n4n5 about 2 months ago
- File docs.patch docs.patch added
Okay, I felt like a decorator could do the trick for decoding functions (see below)
Here is a patch for the docs
def handle_exception(func):
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except Exception as e:
# print(f"An exception occurred: {str(e)}")
return None
return wrapper
Updated by laforge about 2 months ago
n4n5 wrote in #note-5:
Okay, I felt like a decorator could do the trick for decoding functions (see below)
I wonder how this would change the behaviour to the current one? Basically you `edit_binary_decoded`, and then the decoder fails with an exception. Your code suppresses the exception, so you end in the empty editor. You then have to write your new contents in the editor without knowing what the current content is.
So it's not really different from the suggested update_binary_decoded, with the exception that edit_binary_decoded spawns an editor and update_binary_decoded requires you to put the JSON in a single line.
We could probably improve further over that:- catch the decoder exception from within edit_{record,binary}_decoded
- format it as string, add it to the edited file in a comment section on top, including a hexdump of the raw contents
- if the user doesn't make any changes to that text file (just comments), no modification of the file is attempted
- if the user enters valid new data, we encode + write it to the file.
JSON unfortunately doesn't support comments, but we can extend the file format for our purposes.
Here is a patch for the docs
Thanks, I've extended it and submitted it, see https://gerrit.osmocom.org/c/pysim/+/36223
Updated by n4n5 about 2 months ago
As I understand, in the dec_imsi() function, if the imsi is wrong, we return None (except some uncovered case, that my patch could handle)
When we do a edit_binary_decoded it result in this display:
{
"imsi": None
}
that we can edit
The idea on adding a comment is good.
Some python libraries handle json with comments
Anyways, thanks for adding the docs patch
