Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SR-4435] Memory corruption when using NSData.base64EncodedString #4536

Closed
swift-ci opened this issue Mar 30, 2017 · 11 comments
Closed

[SR-4435] Memory corruption when using NSData.base64EncodedString #4536

swift-ci opened this issue Mar 30, 2017 · 11 comments
Labels
bug Foundation Linux

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-4435
Radar None
Original Reporter kallner (JIRA User)
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 4
Component/s Foundation
Labels Bug, Linux
Assignee None
Priority Medium

md5: 890348ccbfdc98261dc31a7e76ba8816

Issue Description:

When one uses NSData.base64EncodedString to convert the contents of an NSData to a base 64 encoded String, memory gets corrupted. The same thing happens if one uses Data.base64EncodedString, as under the covers it invokes NSData.base64EncodedString. This memory corruption can lead to SIGSEGV's and malloc/free related problems.

The following simple program demonstrates the issue:

       import Foundation

       var bytes: [UInt8] = [0xb4, 0xda, 0x5b, 0x80, 0x2f, 0x19, 0x40, 0x33, 0x9c, 0x4a, 
                                       0x41, 0xc0, 0x5a, 0x8a, 0x4a, 0xcd, 0x08, 0xc9, 0xd6, 0x12]

       let data = NSData(bytes: &bytes, length: bytes.count)

       print(data.base64EncodedString(options: .lineLength64Characters))

Compiling it simply with swiftc and running valgrind against it gets the following output:

==10746== Memcheck, a memory error detector
==10746== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==10746== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==10746== Command: ./main
==10746== 
==10746== Invalid write of size 8
==10746==    at 0x56917F9: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746==  Address 0xec21020 is 16 bytes inside a block of size 56 free'd
==10746==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10746==    by 0x568E0C2: _TTSf4n_n_n_d_n___TTSg5GVs22_ContiguousArrayBufferVs5UInt8_GS_S0__s20_ArrayBufferProtocols_GVs14_IgnorePointerS0__GS2_S0__s16_PointerFunctions___TFEsPs20_ArrayBufferProtocol22_arrayOutOfPlaceUpdateuRd__s16_PointerFunctionwd__7Elementzwx7ElementrfTRGVs22_ContiguousArrayBufferwxS2__SiSiqd___T_ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568CDFF: _TTSgq5Vs4Int8___TFVs15ContiguousArray16_copyToNewBufferfT8oldCountSi_T__merged (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x56914A7: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746==  Block was alloc'd at
==10746==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10746==    by 0x5147AB5: swift_slowAlloc (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==10746==    by 0x5147AEE: _swift_allocObject_ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==10746==    by 0x568F2A5: _TTSf4n_n_d___TTSg5Vs5UInt8___TFVs22_ContiguousArrayBufferCfT19_uninitializedCountSi15minimumCapacitySi_GS_x__merged (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x56913EB: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746== 
==10746== Invalid write of size 1
==10746==    at 0x56917FD: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746==  Address 0xec21048 is 0 bytes after a block of size 56 free'd
==10746==    at 0x4C2EDEB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10746==    by 0x568E0C2: _TTSf4n_n_n_d_n___TTSg5GVs22_ContiguousArrayBufferVs5UInt8_GS_S0__s20_ArrayBufferProtocols_GVs14_IgnorePointerS0__GS2_S0__s16_PointerFunctions___TFEsPs20_ArrayBufferProtocol22_arrayOutOfPlaceUpdateuRd__s16_PointerFunctionwd__7Elementzwx7ElementrfTRGVs22_ContiguousArrayBufferwxS2__SiSiqd___T_ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568CDFF: _TTSgq5Vs4Int8___TFVs15ContiguousArray16_copyToNewBufferfT8oldCountSi_T__merged (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x56914A7: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746==  Block was alloc'd at
==10746==    at 0x4C2DB8F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10746==    by 0x5147AB5: swift_slowAlloc (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==10746==    by 0x5147AEE: _swift_allocObject_ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libswiftCore.so)
==10746==    by 0x568F2A5: _TTSf4n_n_d___TTSg5Vs5UInt8___TFVs22_ContiguousArrayBufferCfT19_uninitializedCountSi15minimumCapacitySi_GS_x__merged (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x56913EB: _TTSf4g_n_d___TZFC10Foundation6NSDataP33_6A2A18DBA81B32BFAF1406C41D05FDF517base64EncodeBytesfTGSaVs5UInt8_7optionsVS0_21Base64EncodingOptions_GSaS1__ (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x568AAAA: _TFC10Foundation6NSData19base64EncodedStringfT7optionsVS0_21Base64EncodingOptions_SS (in /home/kallner/swift/swift-3.1-RELEASE-ubuntu16.04/usr/lib/swift/linux/libFoundation.so)
==10746==    by 0x401A72: main (in /home/kallner/bugs/base64EncodedString/main)
==10746== 
tNpbgC8ZQDOcSkHAWopKzQjJhI=
==10746== 
==10746== HEAP SUMMARY:
==10746==     in use at exit: 79,989 bytes in 44 blocks
==10746==   total heap usage: 1,457 allocs, 1,413 frees, 275,270 bytes allocated
==10746== 
==10746== LEAK SUMMARY:
==10746==    definitely lost: 0 bytes in 0 blocks
==10746==    indirectly lost: 0 bytes in 0 blocks
==10746==      possibly lost: 1,808 bytes in 9 blocks
==10746==    still reachable: 78,181 bytes in 35 blocks
==10746==         suppressed: 0 bytes in 0 blocks
==10746== Rerun with --leak-check=full to see details of leaked memory
==10746== 
==10746== For counts of detected and suppressed errors, rerun with: -v
==10746== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
@swift-ci
Copy link
Contributor Author

swift-ci commented Apr 3, 2017

Comment by Shmuel Kallner (JIRA)

It seems that adding a byte of zero to the bytes in the NSData causes the memory corruption to go away. Doing so makes the number of bytes in the NSData a multiple of three. To me this sounds like an error calculating the size of the output buffer.

@parkera
Copy link
Member

parkera commented Apr 11, 2017

This looks like an error in the test case:

import Foundation
var bytes: [UInt8] = [0xb4, 0xda, 0x5b, 0x80, 0x2f, 0x19, 0x40, 0x33, 0x9c, 0x4a, 0x41, 0xc0, 0x5a, 0x8a, 0x4a, 0xcd, 0x08, 0xc9, 0xd6, 0x12]
let data = NSData(bytes: &bytes, length: bytes.count)
// Swift compiler is allowed to free the backing for "bytes" here
print(data.base64EncodedString(options: .lineLength64Characters))

Can you try this instead?

import Foundation
var bytes: [UInt8] = [0xb4, 0xda, 0x5b, 0x80, 0x2f, 0x19, 0x40, 0x33, 0x9c, 0x4a, 0x41, 0xc0, 0x5a, 0x8a, 0x4a, 0xcd, 0x08, 0xc9, 0xd6, 0x12]
let data = Data(bytes: bytes)
print(data.base64EncodedString(options: .lineLength64Characters))

@swift-ci
Copy link
Contributor Author

Comment by Quan (JIRA)

I tried running both versions of the test on Mac and Linux Swift 3.1 and got the following results:

Original test:
Mac: tNpbgC8ZQDOcSkHAWopKzQjJ1hI=
Linux: tNpbgC8ZQDOcSkHAWopKzQjJhI= (NOT SAME RESULT AS MAC)

Suggested fix:
Mac: tNpbgC8ZQDOcSkHAWopKzQjJ1hI=
Linux: *** Error in `/root/test/.build/debug/testPackageTests.xctest': free(): invalid next size (fast): 0x0000000001686bd0 ***

@parkera
Copy link
Member

parkera commented Apr 11, 2017

On second thought, NSData probably would have copied the bytes as soon as it was initialized, so maybe the original bytes array going away shouldn't be a problem.

@swift-ci
Copy link
Contributor Author

Comment by Ian Partridge (JIRA)

This testcase now passes on Linux with the latest master branch of swift-corelibs-foundation.

I am trying to check whether the 3.1 branch is fixed too, but the last 3.1 snapshot was March 27th, so it means building a toolchain from scratch. I'm trying to figure out how the CI builds the 3.1 branch.

@swift-ci
Copy link
Contributor Author

Comment by Ian Partridge (JIRA)

I've verified that this bug is also fixed in the 3.1 branch of swift-corelibs-foundation. The testcase gives the correct output (same as macOS) and valgrind reports no errors.

@swift-ci
Copy link
Contributor Author

Comment by Ian Partridge (JIRA)

We have also verified that the Data fix in the 3.1 branch fixes the issues we've had with Kitura WebSockets and Swift 3.1. I think this one is fixed!

@swift-ci
Copy link
Contributor Author

Comment by Quan (JIRA)

🙂

@parkera
Copy link
Member

parkera commented Apr 12, 2017

Closing this as done since we think it's resolved with the other Data fix on swift-3.1.

@swift-ci
Copy link
Contributor Author

Comment by Bridger Maxwell (JIRA)

Is there a place to download a 3.1 snapshot with this fix? I was hoping to download from https://swift.org/download but I guess the 3.1 branch is no longer being built. Can the build server be nudged to upload another 3.1 build?

@swift-ci
Copy link
Contributor Author

swift-ci commented May 2, 2017

Comment by Ian Partridge (JIRA)

Swift 3.1.1 contains this fix. But I agree that it would be nice to run the 3.1 CI more frequently.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 6, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Foundation Linux
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@parkera @swift-ci