Skip to content

Commit

Permalink
ffi: avoid abi-dependent types. fix more memory leaks and use-after-free
Browse files Browse the repository at this point in the history 
- "bytes" should be `uint8_t*`

- C-strings should be `char*`

- Owned pointers should not be `const`.

- In `native_zxing.h`, clarify the expected ownership of each pointer
  type passed to and returned from the native code. "Owned" pointers must
  to be freed by their callees while "borrowed" pointers must be freed by
  their callers.

- Fix use-after-free bug in `resultToCodeResult` which was returning a
  reference to a dropped `ByteArray` via
  `code->bytes = result.bytes().data()`.
  • Loading branch information
@phlip9
phlip9 committed Jan 31, 2024
1 parent a29369e commit 50793ef
Show file tree
Hide file tree
Showing 7 changed files with 70 additions and 65 deletions.
43 changes: 21 additions & 22 deletions lib/generated_bindings.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ class GeneratedBindings {
: _lookup = lookup;

/// @brief Enables or disables the logging of the library.
/// @param enable Whether to enable or disable the logging.
///
/// @param enabled
/// @param enable Whether to enable or disable the logging.
void setLogEnabled(
int enable,
) {
Expand All @@ -44,7 +43,7 @@ class GeneratedBindings {
late final _setLogEnabled =
_setLogEnabledPtr.asFunction<void Function(int)>();

/// Returns the version of the zxing-cpp library.
/// Returns the version of the zxing-cpp library. Pointer has a static lifetime and must not be freed.
///
/// @return The version of the zxing-cpp library.
ffi.Pointer<ffi.Char> version() {
Expand All @@ -57,7 +56,7 @@ class GeneratedBindings {
_versionPtr.asFunction<ffi.Pointer<ffi.Char> Function()>();

/// @brief Read barcode from image bytes.
/// @param bytes Image bytes.
/// @param bytes Image bytes. Owned pointer. Will be freed by native code.
/// @param imageFormat Image format.
/// @param format Specify a set of BarcodeFormats that should be searched for.
/// @param width Image width in pixels.
Expand All @@ -68,7 +67,7 @@ class GeneratedBindings {
/// @param tryRotate Also try detecting code in 90, 180 and 270 degree rotated images.
/// @return The barcode result.
CodeResult readBarcode(
ffi.Pointer<ffi.Char> bytes,
ffi.Pointer<ffi.Uint8> bytes,
int imageFormat,
int format,
int width,
Expand Down Expand Up @@ -96,7 +95,7 @@ class GeneratedBindings {
late final _readBarcodePtr = _lookup<
ffi.NativeFunction<
CodeResult Function(
ffi.Pointer<ffi.Char>,
ffi.Pointer<ffi.Uint8>,
ffi.Int,
ffi.Int,
ffi.Int,
Expand All @@ -107,11 +106,11 @@ class GeneratedBindings {
ffi.Int,
ffi.Int)>>('readBarcode');
late final _readBarcode = _readBarcodePtr.asFunction<
CodeResult Function(ffi.Pointer<ffi.Char>, int, int, int, int, int, int,
CodeResult Function(ffi.Pointer<ffi.Uint8>, int, int, int, int, int, int,
int, int, int)>();

/// @brief Read barcodes from image bytes.
/// @param bytes Image bytes.
/// @param bytes Image bytes. Owned pointer. Will be freed by native code.
/// @param imageFormat Image format.
/// @param format Specify a set of BarcodeFormats that should be searched for.
/// @param width Image width in pixels.
Expand All @@ -122,7 +121,7 @@ class GeneratedBindings {
/// @param tryRotate Also try detecting code in 90, 180 and 270 degree rotated images.
/// @return The barcode results.
CodeResults readBarcodes(
ffi.Pointer<ffi.Char> bytes,
ffi.Pointer<ffi.Uint8> bytes,
int imageFormat,
int format,
int width,
Expand Down Expand Up @@ -150,7 +149,7 @@ class GeneratedBindings {
late final _readBarcodesPtr = _lookup<
ffi.NativeFunction<
CodeResults Function(
ffi.Pointer<ffi.Char>,
ffi.Pointer<ffi.Uint8>,
ffi.Int,
ffi.Int,
ffi.Int,
Expand All @@ -161,17 +160,17 @@ class GeneratedBindings {
ffi.Int,
ffi.Int)>>('readBarcodes');
late final _readBarcodes = _readBarcodesPtr.asFunction<
CodeResults Function(ffi.Pointer<ffi.Char>, int, int, int, int, int, int,
CodeResults Function(ffi.Pointer<ffi.Uint8>, int, int, int, int, int, int,
int, int, int)>();

/// @brief Encode a string into a barcode
/// @param contents The string to encode
/// @param contents The string to encode. Owned pointer. Will be freed by native code.
/// @param width The width of the barcode in pixels.
/// @param height The height of the barcode in pixels.
/// @param format The format of the barcode
/// @param margin The margin of the barcode
/// @param eccLevel The error correction level of the barcode. Used for Aztec, PDF417, and QRCode only, [0-8].
/// @return The barcode data
/// @return The barcode data.
EncodeResult encodeBarcode(
ffi.Pointer<ffi.Char> contents,
int width,
Expand Down Expand Up @@ -243,18 +242,18 @@ final class Pos extends ffi.Struct {

/// @brief The CodeResult class encapsulates the result of decoding a barcode within an image.
final class CodeResult extends ffi.Struct {
/// < The decoded text
/// < The decoded text. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Char> text;

/// < Whether the barcode was successfully decoded
@ffi.Int()
external int isValid;

/// < The error message
/// < The error message. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Char> error;

/// < The bytes is the raw / standard content without any modifications like character set conversions
external ffi.Pointer<ffi.UnsignedChar> bytes;
/// < The bytes is the raw content without any character set conversions. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Uint8> bytes;

/// < The length of the bytes
@ffi.Int()
Expand Down Expand Up @@ -286,7 +285,7 @@ final class CodeResults extends ffi.Struct {
@ffi.Int()
external int count;

/// < The results of the barcode decoding
/// < The results of the barcode decoding. Owned pointer. Must be freed by Dart code.
external ffi.Pointer<CodeResult> results;

/// < The duration of the decoding in milliseconds
Expand All @@ -300,20 +299,20 @@ final class EncodeResult extends ffi.Struct {
@ffi.Int()
external int isValid;

/// < The encoded text
/// < The encoded text. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Char> text;

/// < The format of the barcode
@ffi.Int()
external int format;

/// < The encoded data
external ffi.Pointer<ffi.SignedChar> data;
/// < The encoded data. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Uint8> data;

/// < The length of the encoded data
@ffi.Int()
external int length;

/// < The error message
/// < The error message. Owned pointer. Must be freed by Dart code if not null.
external ffi.Pointer<ffi.Char> error;
}
2 changes: 1 addition & 1 deletion lib/src/logic/barcode_reader.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Code _readBarcode(
) {
return bindings
.readBarcode(
bytes.allocatePointer(),
bytes.copyToNativePointer(),
params?.imageFormat ?? zx.ImageFormat.lum,
params?.format ?? Format.any,
width,
Expand Down
3 changes: 2 additions & 1 deletion lib/src/logic/barcodes_reader.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ Codes _readBarcodes(
DecodeParams? params,
) {
final CodeResults result = bindings.readBarcodes(
bytes.allocatePointer(),
bytes.copyToNativePointer(),
params?.imageFormat ?? zx.ImageFormat.lum,
params?.format ?? Format.any,
width,
Expand All @@ -81,5 +81,6 @@ Codes _readBarcodes(
for (int i = 0; i < result.count; i++) {
results.add(result.results.elementAt(i).ref.toCode());
}
malloc.free(result.results);
return Codes(codes: results, duration: result.duration);
}
14 changes: 7 additions & 7 deletions lib/src/logic/zxing.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ void setZxingLogEnabled(bool enabled) =>
/// Returns a readable barcode format name
String zxingBarcodeFormatName(int format) => barcodeNames[format] ?? 'Unknown';

extension Uint8ListBlobConversion on Uint8List {
/// Allocates a pointer filled with the Uint8List data.
Pointer<Char> allocatePointer() {
final Pointer<Int8> blob = calloc<Int8>(length);
final Int8List blobBytes = blob.asTypedList(length);
blobBytes.setAll(0, this);
return blob.cast<Char>();
extension Uint8ListExt on Uint8List {
/// Copy the [Uint8List] into a freshly allocated [Pointer<Uint8>].
Pointer<Uint8> copyToNativePointer() {
final Pointer<Uint8> ptr = malloc<Uint8>(length);
final Uint8List view = ptr.asTypedList(length);
view.setAll(0, this);
return ptr;
}
}
8 changes: 3 additions & 5 deletions lib/src/utils/extentions.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import '../../zxing_mobile.dart';
/// From an owned pointer allocated in native code, copy the data into the Dart
/// VM Heap as a [Uint32List] and then immediately `free` the owned ffi pointer.
Uint32List? copyUint32ListFromOwnedFfiPtr(
Pointer<SignedChar> data,
Pointer<Uint8> data,
int length,
) {
if (data == nullptr || length == 0) {
Expand All @@ -23,14 +23,12 @@ Uint32List? copyUint32ListFromOwnedFfiPtr(

/// From an owned pointer allocated in native code, copy the data into the Dart
/// VM Heap as a [Uint8List] and then immediately `free` the owned ffi pointer.
Uint8List? copyUint8ListFromOwnedFfiPtr(
Pointer<UnsignedChar> data, int length) {
Uint8List? copyUint8ListFromOwnedFfiPtr(Pointer<Uint8> data, int length) {
if (data == nullptr || length == 0) {
return null;
}

final Uint8List out =
Uint8List.fromList(data.cast<Int8>().asTypedList(length));
final Uint8List out = Uint8List.fromList(data.asTypedList(length));
malloc.free(data);
return out;
}
Expand Down
26 changes: 14 additions & 12 deletions src/native_zxing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

#include <algorithm>
#include <codecvt>
#include <cstdarg>
#include <cstdlib>
#include <locale>
#include <stdarg.h>

using namespace ZXing;
using namespace std;
Expand All @@ -29,9 +30,11 @@ extern "C"

code->format = static_cast<int>(result.format());

// TODO: this needs to be allocated and coped as well (see text above). Will also require a delete in some flutter code, I assume
code->bytes = result.bytes().data();
code->length = result.bytes().size();
auto length = result.bytes().size();
auto* bytes = new uint8_t[length];
std::copy(result.bytes().begin(), result.bytes().end(), bytes);
code->bytes = bytes;
code->length = length;

auto p = result.position();
auto tl = p.topLeft();
Expand All @@ -58,18 +61,17 @@ extern "C"
}

FUNCTION_ATTRIBUTE
struct CodeResult readBarcode(char *bytes, int imageFormat, int format, int width, int height, int cropWidth, int cropHeight, int tryHarder, int tryRotate, int tryInvert)
struct CodeResult readBarcode(uint8_t* bytes, int imageFormat, int format, int width, int height, int cropWidth, int cropHeight, int tryHarder, int tryRotate, int tryInvert)
{
long long start = get_now();

ImageView image{reinterpret_cast<const uint8_t *>(bytes), width, height, ImageFormat(imageFormat)};
ImageView image {bytes, width, height, ImageFormat(imageFormat)};
if (cropWidth > 0 && cropHeight > 0 && cropWidth < width && cropHeight < height)
{
image = image.cropped(width / 2 - cropWidth / 2, height / 2 - cropHeight / 2, cropWidth, cropHeight);
}
ReaderOptions hints = ReaderOptions().setTryHarder(tryHarder).setTryRotate(tryRotate).setFormats(BarcodeFormat(format)).setTryInvert(tryInvert).setReturnErrors(true);
Result result = ReadBarcode(image, hints);

delete[] bytes;

struct CodeResult code {};
Expand All @@ -84,11 +86,11 @@ extern "C"
}

FUNCTION_ATTRIBUTE
struct CodeResults readBarcodes(char *bytes, int imageFormat, int format, int width, int height, int cropWidth, int cropHeight, int tryHarder, int tryRotate, int tryInvert)
struct CodeResults readBarcodes(uint8_t* bytes, int imageFormat, int format, int width, int height, int cropWidth, int cropHeight, int tryHarder, int tryRotate, int tryInvert)
{
long long start = get_now();

ImageView image{reinterpret_cast<const uint8_t *>(bytes), width, height, ImageFormat(imageFormat)};
ImageView image{bytes, width, height, ImageFormat(imageFormat)};
if (cropWidth > 0 && cropHeight > 0 && cropWidth < width && cropHeight < height)
{
image = image.cropped(width / 2 - cropWidth / 2, height / 2 - cropHeight / 2, cropWidth, cropHeight);
Expand Down Expand Up @@ -117,7 +119,7 @@ extern "C"
}

FUNCTION_ATTRIBUTE
struct EncodeResult encodeBarcode(char *contents, int width, int height, int format, int margin, int eccLevel)
struct EncodeResult encodeBarcode(char* contents, int width, int height, int format, int margin, int eccLevel)
{
long long start = get_now();

Expand All @@ -126,12 +128,12 @@ extern "C"
{
auto writer = MultiFormatWriter(BarcodeFormat(format)).setMargin(margin).setEccLevel(eccLevel).setEncoding(CharacterSet::UTF8);
auto bitMatrix = writer.encode(contents, width, height);
auto matrix = ToMatrix<int8_t>(bitMatrix);
auto matrix = ToMatrix<uint8_t>(bitMatrix);

// We need to return an owned pointer across the ffi boundary. Copy
// the output (again).
auto length = matrix.size();
auto data = new int8_t[length];
auto* data = new uint8_t[length];
std::copy(matrix.begin(), matrix.end(), data);

result.length = length;
Expand Down
Loading

0 comments on commit 50793ef

Please sign in to comment.