Hide Forgot
Created attachment 926222 [details] test program Build attached test case with gcc -g -o pskctest pskctest.c $(pkg-config --cflags --libs libpskc) It loads a HOTP key, bumps the counter, and then attempts to save it. Twice. In pskc_build_xml() we build a new xmlDoc and free the old one... and it looks like we still have references into the old one. ==9202== Memcheck, a memory error detector ==9202== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al. ==9202== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info ==9202== Command: ./pskctest ==9202== Counter: 35 output: <?xml version="1.0" encoding="UTF-8"?> <KeyContainer xmlns="urn:ietf:params:xml:ns:keyprov:pskc" Version="1.0" Id="exampleID1"><KeyPackage><Key Id="12345678" Algorithm="urn:ietf:params:xml:ns:keyprov:pskc:hotp"><Data><Secret><PlainValue>YoTOTvAMztz6JhLQGaoUqhS1f/C0+Jr0B7xIfA0=</PlainValue></Secret><Counter><PlainValue>35</PlainValue></Counter></Data></Key></KeyPackage></KeyContainer> New XML: '<?xml version="1.0"?> <KeyContainer xmlns="urn:ietf:params:xml:ns:keyprov:pskc" Version="1.0" Id="exampleID1"><KeyPackage><Key Id="12345678" Algorithm="urn:ietf:params:xml:ns:keyprov:pskc:hotp"><Data><Secret><PlainValue>YoTOTvAMztz6JhLQGaoUqhS1f/C0+Jr0B7xIfA0=</PlainValue></Secret><Counter><PlainValue>36</PlainValue></Counter></Data></Key></KeyPackage></KeyContainer> ' ==9202== Invalid read of size 1 ==9202== at 0x30554AF0C3: xmlCheckUTF8 (xmlstring.c:788) ==9202== by 0x305545638F: xmlNewPropInternal (tree.c:1869) ==9202== by 0x372E80713B: pskc_build_xml (build.c:444) ==9202== by 0x400BA3: main (pskctest.c:42) ==9202== Address 0x4cb6ee3 is 147 bytes inside a block of size 1,048 free'd ==9202== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==9202== by 0x30554FF679: xmlDictFree (dict.c:809) ==9202== by 0x372E807BB3: pskc_build_xml (build.c:514) ==9202== by 0x400B72: main (pskctest.c:40) ==9202== ==9202== Invalid read of size 1 ==9202== at 0x30554AF0D3: xmlCheckUTF8 (xmlstring.c:788) ==9202== by 0x305545638F: xmlNewPropInternal (tree.c:1869) ==9202== by 0x372E80713B: pskc_build_xml (build.c:444) ==9202== by 0x400BA3: main (pskctest.c:42) ==9202== Address 0x4cb6ee4 is 148 bytes inside a block of size 1,048 free'd ==9202== at 0x4A07577: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==9202== by 0x30554FF679: xmlDictFree (dict.c:809) ==9202== by 0x372E807BB3: pskc_build_xml (build.c:514) ==9202== by 0x400B72: main (pskctest.c:40) ==9202== ...
Created attachment 926224 [details] hack This horrid proof-of-concept hack kind of fixes it for me. It'll obviously fail if there's more than one key in the file, because I deliberately didn't let it reallocate the keypackages array (since the user may have used pskc_get_keypackage() and expect that pointer to continue working). It probably also fails in other ways too, but it demonstrates the cause of the issue quite clearly. Other approaches we could take might be to change the internal pointers as we build up the new XML document, or perhaps just to keep a copy of the *original* XML document from which the pskc_t was parsed, and not free that one.
Created attachment 926225 [details] less horrid, but less efficient fix Actually, retaining the original xmldoc seems like a nice and simple approach. Although it's not as efficient as fixing all the pointers up...
Thanks for the patch. I think the inefficiency shouldn't be problem here and I like it's simple. Applying it in Fedora and forwarded upstream: https://savannah.nongnu.org/support/index.php?108736
oath-toolkit-2.4.1-9.fc21 has been submitted as an update for Fedora 21. https://admin.fedoraproject.org/updates/oath-toolkit-2.4.1-9.fc21
Package oath-toolkit-2.4.1-9.fc21: * should fix your issue, * was pushed to the Fedora 21 testing repository, * should be available at your local mirror within two days. Update it with: # su -c 'yum update --enablerepo=updates-testing oath-toolkit-2.4.1-9.fc21' as soon as you are able to. Please go to the following url: https://admin.fedoraproject.org/updates/FEDORA-2015-1484/oath-toolkit-2.4.1-9.fc21 then log in and leave karma (feedback).
oath-toolkit-2.4.1-9.fc21 has been pushed to the Fedora 21 stable repository. If problems still persist, please make note of it in this bug report.