Skip to content

Minimal change to fix reading 1-past-end of buffer - #4

Open
John-LittleBearLabs wants to merge 1 commit into
lockblox:masterfrom
John-LittleBearLabs:bugfix/3
Open

John-LittleBearLabs wants to merge 1 commit into
lockblox:masterfrom
John-LittleBearLabs:bugfix/3

Conversation

@John-LittleBearLabs

Copy link
Copy Markdown

Correcting boundary check.

Correcting boundary check.
@John-LittleBearLabs

Copy link
Copy Markdown
Author

Here are some affected CIDs, both correct (post-fix) and incorrect (pre-fix) sorted together (the error is always at the end, of course).

Note: this is just what happened on my machine. When you're accessing invalid, possibly uninitialized, memory, your results may very well vary.

bafybeiab6ussnleizgveel2uyig7bl6yxzbgxec7jbcbbqugj53wqq7o23
bafybeiab6ussnleizgveel2uyig7bl6yxzbgxec7jbcbbqugj53wqq7o2y
bafybeiatk2djo7k76lwlyffode33a4vpmleez4vqtrttb6ixyaqv3ixl7e
bafybeiatk2djo7k76lwlyffode33a4vpmleez4vqtrttb6ixyaqv3ixl7h
bafybeiatqn2wouohelg2mfalz6ujmoqdhzp4bnsidhlo2645u27yx2e25u
bafybeiatqn2wouohelg2mfalz6ujmoqdhzp4bnsidhlo2645u27yx2e25x
bafybeib2ouovh24hcizjhnv3xk5wuazwu6urrfgreo4wvyikqg56p6lssq
bafybeib2ouovh24hcizjhnv3xk5wuazwu6urrfgreo4wvyikqg56p6lsst
bafybeibru5jaooun3le664xubcfn37mfa5icmfqpogt4pljye5b3bbhz54
bafybeibru5jaooun3le664xubcfn37mfa5icmfqpogt4pljye5b3bbhz57
bafybeibyjwzzberkr3lutkjjuwlvtmd4k3laipv3vdiaxc3pkni7ake7ha
bafybeibyjwzzberkr3lutkjjuwlvtmd4k3laipv3vdiaxc3pkni7ake7hd
bafybeibypqcpl7qgzdaxzkwd4kgbcyy5jbaqmet64ovtod4zbuzat6chzu
bafybeibypqcpl7qgzdaxzkwd4kgbcyy5jbaqmet64ovtod4zbuzat6chzx
bafybeicmfv6pieekowdzuvpgzqrpwruis5qjllgyjm57cozmjdqe2jyn6i
bafybeicmfv6pieekowdzuvpgzqrpwruis5qjllgyjm57cozmjdqe2jyn6l
bafybeicwl7hsaxdhetfzz4c3txnnocdtkwczxaz2i5mwyc7goraookmbba
bafybeicwl7hsaxdhetfzz4c3txnnocdtkwczxaz2i5mwyc7goraookmbbb
bafybeid7ctk2gcrhjokyljiwq4wyd2usmdg5qlfokyfmyaezemsiwptsme
bafybeid7ctk2gcrhjokyljiwq4wyd2usmdg5qlfokyfmyaezemsiwptsmh
bafybeidfq65jd7lw2awazd2cy3uiclrqjv7patrsqhxdhkmqcxu37ihy7a
bafybeidfq65jd7lw2awazd2cy3uiclrqjv7patrsqhxdhkmqcxu37ihy7d
bafybeidfruh6lnzzvcaru3bffnuxnpvtjeapekomc4ehjhr7myl4n5ppbm
bafybeidfruh6lnzzvcaru3bffnuxnpvtjeapekomc4ehjhr7myl4n5ppbp
bafybeie2pg35ccpcnvsylesq634msypcbhbouwmble6qhzskjwvixqmeou
bafybeie2pg35ccpcnvsylesq634msypcbhbouwmble6qhzskjwvixqmeov
bafybeiepvkj2subyitavspfky7w5nodlvssfaypevimaljf4s3wz32dwb3
bafybeiepvkj2subyitavspfky7w5nodlvssfaypevimaljf4s3wz32dwby
bafybeifqdk6c6le3hn2663ktnabmgcnv447m2epafpokqyxnly65nygndm
bafybeifqdk6c6le3hn2663ktnabmgcnv447m2epafpokqyxnly65nygndn
bafybeig4swjz7dres3n4qfwziufw2d3skzdsvljv64tugdkayrcwb34yjq
bafybeig4swjz7dres3n4qfwziufw2d3skzdsvljv64tugdkayrcwb34yjt
bafybeigi6zad4teafc2krhuhqykbcshe7cfgygve6xhaunkv4wfeuup5i3
bafybeigi6zad4teafc2krhuhqykbcshe7cfgygve6xhaunkv4wfeuup5iy

@markovchainy

Copy link
Copy Markdown
Member

Thanks for the fix!
I've just done a big refactor adding support for many more encodings and also made the code pass my orthodox C++ QC test suite including valgrind.
I think this is no longer an issue but will include the provided CID's in the test to make sure.

@John-LittleBearLabs

Copy link
Copy Markdown
Author

@markovchainy

test suite including valgrind. I think this is no longer an issue but will include the provided CID's in the test to make sure.

BTW I don't think valgrind would've caught it if I stored the bytes in a std::string, since that structure keeps a valid, readable, byte with value zero after the stored bytes.

What might be a better test, not relying on such particularlities, have your own past-the-end byte, and re-run the check with different values in that byte to see if you can get the outcome to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants