philipWendland / IsoApplet

A Java Card PKI Applet aiming to be ISO 7816 compliant
GNU General Public License v3.0
165 stars 72 forks source link

comparing le + offset to file size in processReadBinary #16

Closed hfmanson closed 5 years ago

hfmanson commented 6 years ago

The comparison should use normal integers since le+offset can be greater than 32767. The difference can be cast back to a short.

philipWendland commented 6 years ago

Hi, all smart cards that I know of only support 16 bit integers (signed, because of Java). You can not have files larger than 16 KiB.

Your pull request does not compile:

[cap] error: line 704: net.pwendland.javacard.pki.isoapplet.IsoFileSystem: unsupported int type of intermediate value, must cast intermediate value to type short or byte.
hfmanson commented 6 years ago

Dear philip,

le will be 0x7FFF because the applet implements the ExtendedLength interface

        short le = apdu.setOutgoing();

The problem occurs when reading with a non 0 offset, e.g. 0x100, : in the comparison:

if (le+offset >= fileData.length) {
    le = (short)(fileData.length - offset);
}

Then le+offset is 0x7fff + 0x100 == 0x80FF. When this result is cast to a short it results in a negative number.

Which javac are you using in your project? I converted IsoApplet to Netbeans 8.2 where I don't get a compile error.

hfmanson commented 6 years ago

With this updated code the intermediate value will always be short

philipWendland commented 6 years ago

Squashed, rebased, reworded and applied in 4ec730841b583a57769e676d6511dbf74136f444.

Can I ask what the real-word use case for this was? I cannot imagine why this change is relevant in practice.