Skip to content

Commit

Permalink
Duplicate map label detection for encoding (#209)
Browse files Browse the repository at this point in the history 
This adds duplicate map label detection during encoding as part of sorting.

There was a lot of rework of map sorting. 

UsefulOutBuf_Compare() was changed to behave differently and more universally.


* Duplicate detection for encoding

* rework UsefulOutBuf_Compare and test

* Dup detection seems to be working

* Final tidy-up

---------

Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
  • Loading branch information
@laurencelundblade
laurencelundblade and Laurence Lundblade authored Mar 3, 2024
1 parent 3f34e5a commit dee0d4e
Show file tree
Hide file tree
Showing 6 changed files with 263 additions and 72 deletions.
34 changes: 21 additions & 13 deletions inc/qcbor/UsefulBuf.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* =========================================================================
* Copyright (c) 2016-2018, The Linux Foundation.
* Copyright (c) 2018-2022, Laurence Lundblade.
* Copyright (c) 2018-2024, Laurence Lundblade.
* Copyright (c) 2021, Arm Limited. All rights reserved.
* All rights reserved.
*
Expand Down Expand Up @@ -43,6 +43,7 @@
when who what, where, why
-------- ---- --------------------------------------------------
28/02/2022 llundblade Rearrange UsefulOutBuf_Compare().
19/11/2023 llundblade Add UsefulOutBuf_GetOutput().
19/11/2023 llundblade Add UsefulOutBuf_Swap().
19/11/2023 llundblade Add UsefulOutBuf_Compare().
Expand Down Expand Up @@ -1401,34 +1402,41 @@ UsefulOutBuf_OutUBufOffset(UsefulOutBuf *pUOutBuf, size_t uOffset);
*
* @param[in] pUOutBuf Pointer to the @ref UsefulOutBuf.
* @param[in] uStart1 Offset of first bytes to compare.
* @param[in] uStart2 Offset of second bytes to compare.
* @param[in] uLen1 Length of first bytes to compare.
* @param[in] uStart2 Offset of second bytes to compare.
* @param[in] uLen2 Length of second bytes to compare.
*
* @return 0 for equality, positive if uStart1 is lexographically larger,
* negative if uStart2 is lexographically larger.
*
*
* This looks into bytes that have been output at the offsets @c start1
* and @c start2. It compares bytes at those two starting points until
* they are not equal or the end of the output data is reached from
* one of the starting points.
* they are not equal or @c uLen1 or @c uLen2 is reached. If the
* length of the string given is off the end of the output data, the
* string will be effectively concated to the data in the output
* buffer for the comparison.
*
* This returns positive when @c uStart1 lexographically sorts ahead
* of @c uStart2 and vice versa. Zero is returned if the strings
* compare equally. This only happens when the end of the valid data
* is reached from one of the starting points and the comparison up to
* that point is equality.
* compare equally.
*
* If lengths are unequal and the first bytes are an exact subset of
* the second string, then a positve value will be returned and vice
* versa.
*
* If either start is past the end of data in the output buffer, 0
* will be returned. It is the caller's responsibility to make sure
* the offsets are not off the end such that a comparison is actually
* the offsets are not off the end so that a comparison is actually
* being made. No data will ever be read off the end of the buffer so
* this safe no matter what offsets are passed.
*
* This is a relatively odd function in that it works on data in the
* output buffer. It is employed by QCBOR to sort CBOR-encoded maps that
* are in the output buffer.
* output buffer. It is employed by QCBOR to sort CBOR-encoded maps
* that are in the output buffer.
*/
int UsefulOutBuf_Compare(UsefulOutBuf *pUOutBuf, size_t uStart1, size_t uStart2);

int UsefulOutBuf_Compare(UsefulOutBuf *pUOutBuf,
size_t uStart1, size_t uLen1,
size_t uStart2, size_t uLen2);

/**
* @brief Swap two regions of output bytes.
Expand Down
25 changes: 14 additions & 11 deletions inc/qcbor/qcbor_encode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2188,7 +2188,7 @@ QCBOREncode_OpenMapIndefiniteLengthInMapN(QCBOREncodeContext *pCtx,
* This is the same as QCBOREncode_CloseMap(), but the open map that
* is being close must be of indefinite length.
*/
static void
static void
QCBOREncode_CloseMapIndefiniteLength(QCBOREncodeContext *pCtx);


Expand All @@ -2198,22 +2198,25 @@ QCBOREncode_CloseMapIndefiniteLength(QCBOREncodeContext *pCtx);
* @param[in] pCtx The encoding context to close the map in .
*
* This is the same as QCBOREncode_CloseMap() except it sorts the map
* per RFC 8949 Section 4.2.1. This sort is lexicographic of the CBOR-encoded
* map labels.
* per RFC 8949 Section 4.2.1 and checks for duplicate map keys. This
* sort is lexicographic of the CBOR-encoded map labels.
*
* This is more expensive than most things in the encoder. It uses
* bubble sort which runs in n-squared time where n is the number of
* map items. Sorting large maps on slow CPUs might be slow. This is
* also increases the object code size of the encoder by about 30%
* bubble sort which runs in n-squared time where @c n is the number
* of map items. Sorting large maps on slow CPUs might be slow. This
* is also increases the object code size of the encoder by about 30%
* (500-1000 bytes).
*
* Bubble sort was selected so as to not need an extra buffer to track
* map item offsets. Bubble sort works well even though map items are
* not all the same size because it always swaps adjacent items.
* Bubble sort was selected so as to not need require configuration of
* a buffer to track map item offsets. Bubble sort works well even
* though map items are not all the same size because it always swaps
* adjacent items.
*/
void QCBOREncode_CloseAndSortMap(QCBOREncodeContext *pCtx);
void
QCBOREncode_CloseAndSortMap(QCBOREncodeContext *pCtx);

void QCBOREncode_CloseAndSortMapIndef(QCBOREncodeContext *pCtx);
void
QCBOREncode_CloseAndSortMapIndef(QCBOREncodeContext *pCtx);


/**
Expand Down
30 changes: 24 additions & 6 deletions src/UsefulBuf.c
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*==============================================================================
Copyright (c) 2016-2018, The Linux Foundation.
Copyright (c) 2018-2023, Laurence Lundblade.
Copyright (c) 2018-2024, Laurence Lundblade.
Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions are
Expand Down Expand Up @@ -41,6 +41,7 @@ IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
when who what, where, why
-------- ---- ---------------------------------------------------
28/02/2022 llundblade Rearrange UsefulOutBuf_Compare().
19/11/2023 llundblade Add UsefulOutBuf_GetOutput().
19/11/2023 llundblade Add UsefulOutBuf_Swap().
19/11/2023 llundblade Add UsefulOutBuf_Compare().
Expand Down Expand Up @@ -433,21 +434,27 @@ const void * UsefulInputBuf_GetBytes(UsefulInputBuf *pMe, size_t uAmount)
*
* Code Reviewers: THIS FUNCTION DOES POINTER MATH
*/
int UsefulOutBuf_Compare(UsefulOutBuf *me, size_t uStart1, size_t uStart2)
int UsefulOutBuf_Compare(UsefulOutBuf *pMe,
const size_t uStart1, const size_t uLen1,
const size_t uStart2, const size_t uLen2)
{
const uint8_t *pBase;
const uint8_t *pEnd;
const uint8_t *p1;
const uint8_t *p2;
const uint8_t *p1End;
const uint8_t *p2End;
int uComparison;

pBase = me->UB.ptr;
pEnd = (const uint8_t *)pBase + me->data_len;
pBase = pMe->UB.ptr;
pEnd = (const uint8_t *)pBase + pMe->data_len;
p1 = pBase + uStart1;
p2 = pBase + uStart2;
p1End = p1 + uLen1;
p2End = p2 + uLen2;

uComparison = 0;
while(p1 < pEnd && p2 < pEnd) {
while(p1 < pEnd && p2 < pEnd && p1 < p1End && p2 < p2End) {
uComparison = *p2 - *p1;
if(uComparison != 0) {
break;;
Expand All @@ -456,10 +463,21 @@ int UsefulOutBuf_Compare(UsefulOutBuf *me, size_t uStart1, size_t uStart2)
p2++;
}

if(uComparison == 0 && p1 != p1End && p2 != p2End) {
if(uLen1 > uLen2) {
uComparison = 1;
} else if(uLen2 < uLen1){
uComparison = -1;
} else {
return 0;
}
}

return uComparison;
}



/**
* @brief Reverse order of bytes in a buffer.
*
Expand All @@ -473,7 +491,7 @@ UsefulOutBuf_Private_ReverseBytes(uint8_t *pStart, uint8_t *pEnd)

while(pStart < pEnd) {
pEnd--;
uTmp = *pStart;
uTmp = *pStart;
*pStart = *pEnd;
*pEnd = uTmp;
pStart++;
Expand Down
82 changes: 54 additions & 28 deletions src/qcbor_encode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,7 @@ QCBOR_Private_ConsumeNext(UsefulInputBuf *pInBuf)


/**
* @brief Decoded next item to get its length.
* @brief Decoded next item to get its lengths.
*
* Decode the next item in map no matter what type it is. It works
* recursively when an item is a map or array It returns offset just
Expand All @@ -1299,32 +1299,48 @@ QCBOR_Private_ConsumeNext(UsefulInputBuf *pInBuf)
* stuff that came in from outside. We still want a check for safety
* in case of bugs here, but it is OK to report end of input on error.
*/
static uint32_t
struct ItemLens {
uint32_t uLabelLen;
uint32_t uItemLen;
};

static struct ItemLens
QCBOREncode_Private_DecodeNextInMap(QCBOREncodeContext *pMe, uint32_t uStart)
{
UsefulInputBuf InBuf;
UsefulBufC EncodedMapBytes;
QCBORError uCBORError;
UsefulInputBuf InBuf;
UsefulBufC EncodedMapBytes;
QCBORError uCBORError;
struct ItemLens Result;

Result.uLabelLen = 0;
Result.uItemLen = 0;

EncodedMapBytes = UsefulOutBuf_OutUBufOffset(&(pMe->OutBuf), uStart);
if(UsefulBuf_IsNULLC(EncodedMapBytes)) {
return 0;
return Result;
}

UsefulInputBuf_Init(&InBuf, EncodedMapBytes);

/* This is always used on maps, so consume two, the label and the value */
uCBORError = QCBOR_Private_ConsumeNext(&InBuf);
if(uCBORError) {
return 0;
return Result;
}

/* Cast is safe because this is QCBOR which limits sizes to UINT32_MAX */
Result.uLabelLen = (uint32_t)UsefulInputBuf_Tell(&InBuf);

uCBORError = QCBOR_Private_ConsumeNext(&InBuf);
if(uCBORError) {
return 0;
Result.uLabelLen = 0;
return Result;
}

Result.uItemLen = (uint32_t)UsefulInputBuf_Tell(&InBuf);

/* Cast is safe because this is QCBOR which limits sizes to UINT32_MAX */
return (uint32_t)UsefulInputBuf_Tell(&InBuf);
return Result;
}


Expand All @@ -1342,12 +1358,13 @@ QCBOREncode_Private_DecodeNextInMap(QCBOREncodeContext *pMe, uint32_t uStart)
static void
QCBOREncode_Private_SortMap(QCBOREncodeContext *pMe, uint32_t uStart)
{
bool bSwapped;
int nComparison;
uint32_t uLen2;
uint32_t uLen1;
uint32_t uStart1;
uint32_t uStart2;
bool bSwapped;
int nComparison;
uint32_t uStart1;
uint32_t uStart2;
struct ItemLens Lens1;
struct ItemLens Lens2;


if(pMe->uError != QCBOR_SUCCESS) {
return;
Expand All @@ -1367,31 +1384,38 @@ QCBOREncode_Private_SortMap(QCBOREncodeContext *pMe, uint32_t uStart)
* sizes are not the same and overlap may occur in the bytes being
* swapped.
*/
do {
uLen1 = QCBOREncode_Private_DecodeNextInMap(pMe, uStart);
if(uLen1 == 0) {
do { /* Loop until nothing was swapped */
Lens1 = QCBOREncode_Private_DecodeNextInMap(pMe, uStart);
if(Lens1.uLabelLen == 0) {
/* It's an empty map. Nothing to do. */
break;
}
uStart1 = uStart;
uStart2 = uStart1 + uLen1;
uStart2 = uStart1 + Lens1.uItemLen;
bSwapped = false;

while(1) {
uLen2 = QCBOREncode_Private_DecodeNextInMap(pMe, uStart2);
if(uLen2 == 0) {
Lens2 = QCBOREncode_Private_DecodeNextInMap(pMe, uStart2);
if(Lens2.uLabelLen == 0) {
break;
}

nComparison = UsefulOutBuf_Compare(&(pMe->OutBuf), uStart1, uStart2);
nComparison = UsefulOutBuf_Compare(&(pMe->OutBuf),
uStart1, Lens1.uLabelLen,
uStart2, Lens2.uLabelLen);
if(nComparison < 0) {
UsefulOutBuf_Swap(&(pMe->OutBuf), uStart1, uStart2, uStart2 + uLen2);
uStart1 = uStart1 + uLen2;
UsefulOutBuf_Swap(&(pMe->OutBuf), uStart1, uStart2, uStart2 + Lens2.uItemLen);
uStart1 = uStart1 + Lens2.uItemLen; /* item 2 now in position of item 1 */
/* Lens1 is still valid as Lens1 for the next loop */
bSwapped = true;
} else {
} else if(nComparison > 0) {
uStart1 = uStart2;
Lens1 = Lens2;
} else /* nComparison == 0 */ {
pMe->uError = QCBOR_ERR_DUPLICATE_LABEL;
return;
}
uStart2 = uStart2 + uLen2;
uStart2 = uStart2 + Lens2.uItemLen;
}
} while(bSwapped);
}
Expand All @@ -1400,7 +1424,8 @@ QCBOREncode_Private_SortMap(QCBOREncodeContext *pMe, uint32_t uStart)
/*
* Public functions for closing sorted maps. See qcbor/qcbor_encode.h
*/
void QCBOREncode_CloseAndSortMap(QCBOREncodeContext *pMe)
void
QCBOREncode_CloseAndSortMap(QCBOREncodeContext *pMe)
{
uint32_t uStart;

Expand All @@ -1419,7 +1444,8 @@ void QCBOREncode_CloseAndSortMap(QCBOREncodeContext *pMe)
/*
* Public functions for closing sorted maps. See qcbor/qcbor_encode.h
*/
void QCBOREncode_CloseAndSortMapIndef(QCBOREncodeContext *pMe)
void
QCBOREncode_CloseAndSortMapIndef(QCBOREncodeContext *pMe)
{
uint32_t uStart;

Expand Down
Loading

0 comments on commit dee0d4e

Please sign in to comment.