oblador / react-native-keychain

:key: Keychain Access for React Native
MIT License
3.17k stars 520 forks source link

Writing more then 65kb results in corrupted data #184

Open ulfgebhardt opened 5 years ago

ulfgebhardt commented 5 years ago

Bugreport

Writing more then 65kb results in corrupted data

reading the corrupt data results in Unexpected token [...] in JSON at position 65536

Data written

image

Data read

image

The Code

image Writing 30k Integers into the buffer

More Context

image

The Error

image

The Version

"react-native-keychain": "^3.0.0-rc.3"

react-native-keychain@^3.0.0-rc.3:
  version "3.0.0"
  resolved "https://registry.yarnpkg.com/react-native-keychain/-/react-native-keychain-3.0.0.tgz#29da1dfa43c2581f76bf9420914fd38a1558cf18"
  integrity sha512-0incABt1+aXsZvG34mDV57KKanSB+iMHmxvWv+N6lgpNLaSoqrCxazjbZdeqD4qJ7Z+Etp5CLf/4v1aI+sNLBw==

The Device

A real Android 6.0 Device using react-native debug mode(maybe this causes trouble?)

image

The Current Solution

Split up the data into buckets, have another key which indexes the buckets.

This reduces speed, since there are more read/write Operations.

Assume:

Results in ceil(n/b) + 1 read/write operations

image

Note: since its not easy to determine the size of an Object in byte, you have to select a size for the buckets which you consider save not to surpass 65kb.

The Current Solution's Code

Sorry for that mess, cleanup is not yet done - ping me here for a followup ;-)

static readKeychain = async () => {
    console.log('readKeychain');
    const keyindex = await Keychain.getGenericPassword(VotesLocal.KEYCHAIN_INDEX_SERVICE);
    if (!keyindex) {
      return null;
    }
    let indexchain = JSON.parse(keyindex.password);
    console.log(`readKeychain: ${JSON.stringify(indexchain)}`);
    indexchain.d = [];
    await Promise.all(
      indexchain.i.map(async serviceId => {
        const service = VotesLocal.KEYCHAIN_VOTES_SERVICE + serviceId;
        const setData = await Keychain.getGenericPassword(service);
        console.log(`readKeychain: ${JSON.stringify(service)}`);
        console.log(`readKeychain: ${JSON.stringify(setData)}`);
        if (setData) {
          indexchain.d.push(...JSON.parse(setData.password));
        }
      }),
    );
    delete indexchain.i;
    console.log(`readKeychain: ${JSON.stringify(indexchain)}`);
    return indexchain;
  };

  static writeKeychain = async data => {
    console.log('writeKeychain');
    console.log(`writeKeychain: ${JSON.stringify(data)}`);
    let index = [];
    // Split Data into packages to avoid error on 65k
    while (data.d.length > 0) {
      const set = data.d.splice(0, VotesLocal.KEYCHAIN_MAXSIZE);
      const setServiceId = index.length;
      const setService = VotesLocal.KEYCHAIN_VOTES_SERVICE + setServiceId;
      console.log(`writeKeychain: ${JSON.stringify(setService)}`);
      console.log(`writeKeychain: ${JSON.stringify(set)}`);
      await Keychain.setGenericPassword(
        VotesLocal.KEYCHAIN_VOTES_KEY,
        JSON.stringify(set),
        setService,
      );
      index.push(setServiceId);
    }

    delete data.d;
    data.i = index;
    console.log(`writeKeychain: ${JSON.stringify(data)}`);
    // console.log(JSON.stringify(data));
    /*const dummy = [];
    for (var i = 0; i < 30000; i++) {
      dummy.push(i);
    }
    data.dummy = dummy;
    console.log(data);
    console.log(JSON.stringify(data));*/

    return await Keychain.setGenericPassword(
      VotesLocal.KEYCHAIN_INDEX_KEY,
      JSON.stringify(data),
      VotesLocal.KEYCHAIN_INDEX_SERVICE,
    );
  };

Furthermore

Since the corruption in the data seem to be random bytes (changing values), I consider it likely that this is a bufferoverflow?!

And yes - this library is made to contain credentials not data - I know that we abuse its scope ;-)

(Possibly) Related

https://github.com/oblador/react-native-keychain/issues/120 https://github.com/oblador/react-native-keychain/issues/174 https://github.com/oblador/react-native-keychain/issues/208

vonovak commented 5 years ago

Thanks for reporting. Specifying the platform and adding a repro would be nice. I will not have time to look into this though. Thanks.

ulfgebhardt commented 5 years ago

@vonovak Done - consider testing it out - its a simple while loop you have to put into your Write-KeyChain-Code

Edit: https://github.com/demokratie-live/democracy-client/blob/3d2aee279d4ad617240bb6cb5a23e7a8f0ee54e2/src/services/VotesLocal.js#L93

suvro24 commented 5 years ago

is it fixed in the latest version?

sebk commented 5 years ago

I still have the same problem and also with the latest library versions the problem still exists.

Has somebody else an idea howto solve it instead splitting the reading and writing?

I guess it is also the same as https://github.com/oblador/react-native-keychain/issues/208 (?)

blankey1337 commented 4 years ago

Just ran into this on Android 7.

quicksilverr commented 3 years ago

Hey Guys,

I just across the same issue, even in the version 7.0.0. I'm using RSA as the crypto option for android, without which a biometric prompt isn't displayed. But I'm getting string size error. Thanks @ulfgebhardt for the solution to such a complex problem.

I was wondering, can we compress our string token and then pass it as a token to setGenericPassword and then decompress that token when it is returned from getGenericPassword? Is that possible and advisable?