hf mfu dump returns corrupt data

[elafargue]

I noticed an issue with hf mdu dump: on an Ultralight EV1 48 bytes, block 15 returns a wrong value with the first two bytes at zero, where the first two bytes have a non-zero value on the card. These two bytes are always dumps as having a zero value with all the MFU cards I read, but this is not correct, as the dump below shows:

hf mfu dump
TYPE : MIFARE Ultralight EV1 48bytes (MF0UL1101)          
Reading tag memory...          
#db# Cmd Error: 00          
#db# Read block 16 error          
Authentication Failed UL-EV1/NTAG          
*special* data

......
13/0x0D | 36 36 36 36 | 0 | 6666          
 14/0x0E | 36 36 36 00 | 0 | 666.          
 15/0x0F | 00 00 AE D9 | 0 | ....          
---------------------------------          
Dumped 28 pages, wrote 112 bytes to XXXXXX.bin          
pm3 --> hf mfu rdbl b 15
Block#  | Data        | Ascii

-----------------------------          
15/0x0F | EE A3 AE D9 | ....

This behavior is not present in the official repository. I checked that the correct value for block 15 is "EE A3 AE D9" and not "00 00 AE D9".

[elafargue]

I lack the time to do a proper PR (apologies) but I found the cause of the problem:

The dump command assumes wrongly that all card pages were read, which will fail for the config block when we don't have the password, and it overwrites the first two bytes of the last page that was read with 0x00, 0x00. If all pages were read, we're OK, but if that is not the case, then we are overwriting either page 15 or page 35, hence the bug. Makes sense?

Not 100% sure of the best way to patch this. The check below works for me, but there might be better ways? I don't have a blank EV1 where I know the password so I can't check all use-cases.

diff --git a/client/cmdhfmfu.c b/client/cmdhfmfu.c
index 15e2f3d3..54ec9d1a 100644
--- a/client/cmdhfmfu.c
+++ b/client/cmdhfmfu.c
@@ -1817,10 +1817,12 @@ int CmdHF14AMfUDump(const char *Cmd){
                        //reset pack
                        get_pack[0]=0;
                        get_pack[1]=0;
+               } else {
+                       // add pack to block read only if ul_auth_select was successful
+                       // so that we don't overwrite page 15 or page 35
+                       memcpy(data + (pages*4) - 4, get_pack, sizeof(get_pack));
                }
                DropField();
-               // add pack to block read               
-               memcpy(data + (pages*4) - 4, get_pack, sizeof(get_pack));

                if ( hasAuthKey )
                        ul_auth_select( &card, tagtype, hasAuthKey, authKeyPtr, dummy_pack, sizeof(dummy_pack));

[iceman1001]

No, you failed to understand the differences, there is a bug but icemanfork is correct, offical pm3 has a bug, where it faulty thinks it can "read" the PACK from memory. Only way to get PACK is to perform a authentication with correct pwd. That is why you see the pack bytes being cleared if the authentication fails.

[elafargue]

Yes, I saw that you correctly read the PACK, but the issue is that in your code, the clearing of pack when there is no successful auth overwrites the first two bytes of page 15, not of page 19 where pack lives, because the PM3 stops reading after page 15 when auth fails. So your code overwrites a correctly read user memory page... Try it!

Ed

On Mar 15, 2018 23:10, "Iceman" notifications@github.com wrote:

No, you failed to understand the differences, there is a bug but icemanfork is correct, offical pm3 has a bug, where it faulty thinks it can "read" the PACK from memory. Only way to get PACK is to perform a authentication with correct pwd. That is why you see the pack bytes being cleared if the authentication fails.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/iceman1001/proxmark3/issues/206#issuecomment-373613311, or mute the thread https://github.com/notifications/unsubscribe-auth/ABH4B7jZMpCM1DqgZ2m7sEPANXlNLxbuks5te1dNgaJpZM4StFSz .

[iceman1001]

Now I see the cause. Yes, there is a bug. The problem is not the pack data itself. The problem lays earlier when the variable pages get set from read data. If reading data fails (because of failed authentication), the total length becomes 4 block shorter in your case where it fails from reading from block 16 and forward.

pages = bufferSize/4;

The first natural patch would be changing to default pages for found card, however, I suspect that will influence a dump with only partial data to be written. As it does now, it writes found data only.

So either the dumps will be of total length of card memory but have zeros on places where it shouldnt, like config blocks. Or dumps will be only as long as current read data and notify its failure to make a complete dump.

I'm not sure I like any of the two solutions.

Looking in the next step, when used with hf mfu restore a dump with bad config data will be devasting to card.

[elafargue]

Yes, exactly - I would tend to go for the least bad solution: dump all the pages that can be read (0 to 15 in this case) and write a warning on the proxmark3 utility that this was a partial dump due to the lack of valid authentication? If should be easy to check the number of pages returned and detect if this is a value that is too short.

In my case, this kind of partial dump of the user memory pages was all I needed to simulate a particular MFU card, so that was actually useful :) You can still load a partial dump into the simulator and it behaves correctly.

[iceman1001]

indeed, check and warning for partial dump. This bug also influences when manually setting the number pages to be read.

True, simulation don't care too much about acting according to configuration :)

[iceman1001]

I pushed a fix, try it and see if that solves the different use-cases,

1. partial read 2.. partial read because of manually set

[elafargue]

Yes, it looks like you fix works well, no data corruption in the (partial) reads I did. I would also add a clear warning a the end of the dump stating that the dump is partial and the user should be careful.

[iceman1001]

Feel free to change the message.

All that is lacking is full dump tests.