packages/mozjs78
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
mozjs78/backport-CVE-2023-29532.patch
Jiayi Yin ef187d898a init
2025-03-17 06:21:03 +00:00

1202 lines
44 KiB
Diff
Raw Blame History

From 1648665733ed9c26ce1f23acb5fa1362623de6c6 Mon Sep 17 00:00:00 2001
From: User Robin Steuber <bytesized@mozilla.com>
Date: Thu, 30 May 2024 15:14:51 +0800
Subject: [PATCH] CVE-2023-29532
Conflict:NA
Reference:https://bug1806394.bmoattachments.org/attachment.cgi?id=9318233
---
modules/libmar/src/mar.h | 63 ++++-
modules/libmar/src/mar_extract.c | 4 +-
modules/libmar/src/mar_read.c | 259 +++++++++++++-----
modules/libmar/tool/mar.c | 9 +-
modules/libmar/verify/mar_verify.c | 143 +++++-----
toolkit/mozapps/update/UpdateService.jsm | 32 ++-
toolkit/mozapps/update/common/updatererrors.h | 6 +
.../mozapps/update/tests/browser/browser.ini | 3 +
...rowser_memory_allocation_error_fallback.js | 81 ++++++
.../mozapps/update/updater/archivereader.cpp | 9 +-
toolkit/mozapps/update/updater/updater.cpp | 6 +
11 files changed, 454 insertions(+), 161 deletions(-)
create mode 100644 toolkit/mozapps/update/tests/browser/browser_memory_allocation_error_fallback.js
diff --git a/modules/libmar/src/mar.h b/modules/libmar/src/mar.h
index 4f9ae487e7..74ab7657f8 100644
--- a/modules/libmar/src/mar.h
+++ b/modules/libmar/src/mar.h
@@ -55,7 +55,8 @@ typedef struct SeenIndex_ {
* Mozilla ARchive (MAR) file data structure
*/
struct MarFile_ {
- FILE* fp; /* file pointer to the archive */
+ unsigned char* buffer; /* file buffer containing the entire MAR */
+ size_t data_len; /* byte count of the data in the buffer */
MarItem* item_table[TABLESIZE]; /* hash table of files in the archive */
SeenIndex* index_list; /* file indexes processed */
int item_table_is_valid; /* header and index validation flag */
@@ -72,16 +73,29 @@ typedef struct MarFile_ MarFile;
*/
typedef int (*MarItemCallback)(MarFile* mar, const MarItem* item, void* data);
+enum MarReadResult_ {
+ MAR_READ_SUCCESS,
+ MAR_IO_ERROR,
+ MAR_MEM_ERROR,
+ MAR_FILE_TOO_BIG_ERROR,
+};
+
+typedef enum MarReadResult_ MarReadResult;
+
/**
* Open a MAR file for reading.
* @param path Specifies the path to the MAR file to open. This path must
* be compatible with fopen.
+ * @param out_mar Out-parameter through which the created MarFile structure is
+ * returned. Guaranteed to be a valid structure if
+ * MAR_READ_SUCCESS is returned. Otherwise NULL will be
+ * assigned.
* @return NULL if an error occurs.
*/
-MarFile* mar_open(const char* path);
+MarReadResult mar_open(const char* path, MarFile** out_mar);
#ifdef XP_WIN
-MarFile* mar_wopen(const wchar_t* path);
+MarReadResult mar_wopen(const wchar_t* path, MarFile** out_mar);
#endif
/**
@@ -90,6 +104,49 @@ MarFile* mar_wopen(const wchar_t* path);
*/
void mar_close(MarFile* mar);
+/**
+ * Reads the specified amount of data from the buffer in MarFile that contains
+ * the entirety of the MAR file data.
+ * @param mar The MAR file to read from.
+ * @param dest The buffer to read into.
+ * @param position The byte index to start reading from the MAR at.
+ * On success, position will be incremented by size.
+ * @param size The number of bytes to read.
+ * @return 0 If the specified amount of data was read.
+ * -1 If the buffer MAR is not large enough to read the
+ * specified amount of data at the specified position.
+ */
+int mar_read_buffer(MarFile* mar, void* dest, size_t* position, size_t size);
+
+/**
+ * Reads the specified amount of data from the buffer in MarFile that contains
+ * the entirety of the MAR file data. If there isn't that much data remaining,
+ * reads as much as possible.
+ * @param mar The MAR file to read from.
+ * @param dest The buffer to read into.
+ * @param position The byte index to start reading from the MAR at.
+ * This function will increment position by the number of bytes
+ * copied.
+ * @param size The maximum number of bytes to read.
+ * @return The number of bytes copied into dest.
+ */
+int mar_read_buffer_max(MarFile* mar, void* dest, size_t* position,
+ size_t size);
+
+/**
+ * Increments position by distance. Checks that the resulting position is still
+ * within the bounds of the buffer. Much like fseek, this will allow position to
+ * be successfully placed just after the end of the buffer.
+ * @param mar The MAR file to read from.
+ * @param position The byte index to start reading from the MAR at.
+ * On success, position will be incremented by size.
+ * @param distance The number of bytes to move forward by.
+ * @return 0 If position was successfully moved.
+ * -1 If moving position by distance would move it outside the
+ * bounds of the buffer.
+ */
+int mar_buffer_seek(MarFile* mar, size_t* position, size_t distance);
+
/**
* Find an item in the MAR file by name.
* @param mar The MarFile object to query.
diff --git a/modules/libmar/src/mar_extract.c b/modules/libmar/src/mar_extract.c
index 28693e24b0..9b4a3fa26d 100644
--- a/modules/libmar/src/mar_extract.c
+++ b/modules/libmar/src/mar_extract.c
@@ -75,8 +75,8 @@ int mar_extract(const char* path) {
MarFile* mar;
int rv;
- mar = mar_open(path);
- if (!mar) {
+ MarReadResult result = mar_open(path, &mar);
+ if (result != MAR_READ_SUCCESS) {
return -1;
}
diff --git a/modules/libmar/src/mar_read.c b/modules/libmar/src/mar_read.c
index 883760903d..fb743151f4 100644
--- a/modules/libmar/src/mar_read.c
+++ b/modules/libmar/src/mar_read.c
@@ -112,25 +112,28 @@ static int mar_consume_index(MarFile* mar, char** buf, const char* buf_end) {
static int mar_read_index(MarFile* mar) {
char id[MAR_ID_SIZE], *buf, *bufptr, *bufend;
uint32_t offset_to_index, size_of_index;
+ size_t mar_position = 0;
/* verify MAR ID */
- fseek(mar->fp, 0, SEEK_SET);
- if (fread(id, MAR_ID_SIZE, 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, id, &mar_position, MAR_ID_SIZE) != 0) {
return -1;
}
if (memcmp(id, MAR_ID, MAR_ID_SIZE) != 0) {
return -1;
}
- if (fread(&offset_to_index, sizeof(uint32_t), 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, &offset_to_index, &mar_position,
+ sizeof(uint32_t)) != 0) {
return -1;
}
offset_to_index = ntohl(offset_to_index);
- if (fseek(mar->fp, offset_to_index, SEEK_SET)) {
+ mar_position = 0;
+ if (mar_buffer_seek(mar, &mar_position, offset_to_index) != 0) {
return -1;
}
- if (fread(&size_of_index, sizeof(uint32_t), 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, &size_of_index, &mar_position,
+ sizeof(uint32_t)) != 0) {
return -1;
}
size_of_index = ntohl(size_of_index);
@@ -139,7 +142,7 @@ static int mar_read_index(MarFile* mar) {
if (!buf) {
return -1;
}
- if (fread(buf, size_of_index, 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, buf, &mar_position, size_of_index) != 0) {
free(buf);
return -1;
}
@@ -210,50 +213,96 @@ static int mar_insert_offset(MarFile* mar, uint32_t offset, uint32_t length) {
/**
* Internal shared code for mar_open and mar_wopen.
- * On failure, will fclose(fp).
+ * Reads the entire MAR into memory. Fails if it is bigger than
+ * MAX_SIZE_OF_MAR_FILE bytes.
*/
-static MarFile* mar_fpopen(FILE* fp) {
+static MarReadResult mar_fpopen(FILE* fp, MarFile** out_mar) {
+ *out_mar = NULL;
MarFile* mar;
mar = (MarFile*)malloc(sizeof(*mar));
if (!mar) {
- fclose(fp);
- return NULL;
+ return MAR_MEM_ERROR;
+ }
+
+ off_t buffer_size = -1;
+ if (fseeko(fp, 0, SEEK_END) == 0) {
+ buffer_size = ftello(fp);
+ }
+ rewind(fp);
+ if (buffer_size < 0) {
+ fprintf(stderr, "Warning: MAR size could not be determined\n");
+ buffer_size = MAX_SIZE_OF_MAR_FILE;
+ }
+ if (buffer_size > MAX_SIZE_OF_MAR_FILE) {
+ fprintf(stderr, "ERROR: MAR exceeds maximum size (%lli)\n",
+ (long long int)buffer_size);
+ free(mar);
+ return MAR_FILE_TOO_BIG_ERROR;
+ }
+
+ mar->buffer = malloc(buffer_size);
+ if (!mar->buffer) {
+ fprintf(stderr, "ERROR: MAR buffer could not be allocated\n");
+ free(mar);
+ return MAR_MEM_ERROR;
+ }
+ mar->data_len = fread(mar->buffer, 1, buffer_size, fp);
+ if (fgetc(fp) != EOF) {
+ fprintf(stderr, "ERROR: File is larger than buffer (%lli)\n",
+ (long long int)buffer_size);
+ free(mar->buffer);
+ free(mar);
+ return MAR_IO_ERROR;
+ }
+ if (ferror(fp)) {
+ fprintf(stderr, "ERROR: Failed to read MAR\n");
+ free(mar->buffer);
+ free(mar);
+ return MAR_IO_ERROR;
}
- mar->fp = fp;
mar->item_table_is_valid = 0;
memset(mar->item_table, 0, sizeof(mar->item_table));
mar->index_list = NULL;
- return mar;
+ *out_mar = mar;
+ return MAR_READ_SUCCESS;
}
-MarFile* mar_open(const char* path) {
+MarReadResult mar_open(const char* path, MarFile** out_mar) {
+ *out_mar = NULL;
+
FILE* fp;
fp = fopen(path, "rb");
if (!fp) {
fprintf(stderr, "ERROR: could not open file in mar_open()\n");
perror(path);
- return NULL;
+ return MAR_IO_ERROR;
}
- return mar_fpopen(fp);
+ MarReadResult result = mar_fpopen(fp, out_mar);
+ fclose(fp);
+ return result;
}
#ifdef XP_WIN
-MarFile* mar_wopen(const wchar_t* path) {
+MarReadResult mar_wopen(const wchar_t* path, MarFile** out_mar) {
+ *out_mar = NULL;
+
FILE* fp;
_wfopen_s(&fp, path, L"rb");
if (!fp) {
fprintf(stderr, "ERROR: could not open file in mar_wopen()\n");
_wperror(path);
- return NULL;
+ return MAR_IO_ERROR;
}
- return mar_fpopen(fp);
+ MarReadResult result = mar_fpopen(fp, out_mar);
+ fclose(fp);
+ return result;
}
#endif
@@ -262,7 +311,7 @@ void mar_close(MarFile* mar) {
SeenIndex* index;
int i;
- fclose(mar->fp);
+ free(mar->buffer);
for (i = 0; i < TABLESIZE; ++i) {
item = mar->item_table[i];
@@ -282,10 +331,61 @@ void mar_close(MarFile* mar) {
free(mar);
}
+int mar_read_buffer(MarFile* mar, void* dest, size_t* position, size_t size) {
+ // size may be provided by the MAR, which we may not have finished validating
+ // the signature on yet. Make sure not to trust it in a way that could
+ // cause an overflow.
+ if (size > mar->data_len) {
+ return -1;
+ }
+ if (*position > mar->data_len - size) {
+ return -1;
+ }
+ memcpy(dest, mar->buffer + *position, size);
+ *position += size;
+ return 0;
+}
+
+int mar_read_buffer_max(MarFile* mar, void* dest, size_t* position,
+ size_t size) {
+ // size may be provided by the MAR, which we may not have finished validating
+ // the signature on yet. Make sure not to trust it in a way that could
+ // cause an overflow.
+ if (mar->data_len <= *position) {
+ return 0;
+ }
+ size_t read_count = mar->data_len - *position;
+ if (read_count > size) {
+ read_count = size;
+ }
+ memcpy(dest, mar->buffer + *position, read_count);
+ *position += read_count;
+ return read_count;
+}
+
+int mar_buffer_seek(MarFile* mar, size_t* position, size_t distance) {
+ // distance may be provided by the MAR, which we may not have finished
+ // validating the signature on yet. Make sure not to trust it in a way that
+ // could cause an overflow.
+ if (distance > mar->data_len) {
+ return -1;
+ }
+ if (*position > mar->data_len - distance) {
+ return -1;
+ }
+ *position += distance;
+ return 0;
+}
+
/**
* Determines the MAR file information.
*
- * @param fp An opened MAR file in read mode.
+ * @param mar An open MAR file.
+ * @param mar_position The current position in the MAR.
+ * Its value will be updated to the current
+ * position in the MAR after the function exits.
+ * Since its initial value will never actually be
+ * used, this is effectively an outparam.
* @param hasSignatureBlock Optional out parameter specifying if the MAR
* file has a signature block or not.
* @param numSignatures Optional out parameter for storing the number
@@ -300,10 +400,11 @@ void mar_close(MarFile* mar) {
* hasAdditionalBlocks is not equal to 0.
* @return 0 on success and non-zero on failure.
*/
-int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
- uint32_t* numSignatures, int* hasAdditionalBlocks,
- uint32_t* offsetAdditionalBlocks,
- uint32_t* numAdditionalBlocks) {
+int get_open_mar_file_info(MarFile* mar, size_t* mar_position,
+ int* hasSignatureBlock, uint32_t* numSignatures,
+ int* hasAdditionalBlocks,
+ uint32_t* offsetAdditionalBlocks,
+ uint32_t* numAdditionalBlocks) {
uint32_t offsetToIndex, offsetToContent, signatureCount, signatureLen, i;
/* One of hasSignatureBlock or hasAdditionalBlocks must be non NULL */
@@ -312,24 +413,27 @@ int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
}
/* Skip to the start of the offset index */
- if (fseek(fp, MAR_ID_SIZE, SEEK_SET)) {
+ *mar_position = 0;
+ if (mar_buffer_seek(mar, mar_position, MAR_ID_SIZE) != 0) {
return -1;
}
/* Read the offset to the index. */
- if (fread(&offsetToIndex, sizeof(offsetToIndex), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &offsetToIndex, mar_position,
+ sizeof(offsetToIndex)) != 0) {
return -1;
}
offsetToIndex = ntohl(offsetToIndex);
if (numSignatures) {
/* Skip past the MAR file size field */
- if (fseek(fp, sizeof(uint64_t), SEEK_CUR)) {
+ if (mar_buffer_seek(mar, mar_position, sizeof(uint64_t)) != 0) {
return -1;
}
/* Read the number of signatures field */
- if (fread(numSignatures, sizeof(*numSignatures), 1, fp) != 1) {
+ if (mar_read_buffer(mar, numSignatures, mar_position,
+ sizeof(*numSignatures)) != 0) {
return -1;
}
*numSignatures = ntohl(*numSignatures);
@@ -337,17 +441,19 @@ int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
/* Skip to the first index entry past the index size field
We do it in 2 calls because offsetToIndex + sizeof(uint32_t)
- could oerflow in theory. */
- if (fseek(fp, offsetToIndex, SEEK_SET)) {
+ could overflow in theory. */
+ *mar_position = 0;
+ if (mar_buffer_seek(mar, mar_position, offsetToIndex) != 0) {
return -1;
}
- if (fseek(fp, sizeof(uint32_t), SEEK_CUR)) {
+ if (mar_buffer_seek(mar, mar_position, sizeof(uint32_t)) != 0) {
return -1;
}
/* Read the first offset to content field. */
- if (fread(&offsetToContent, sizeof(offsetToContent), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &offsetToContent, mar_position,
+ sizeof(offsetToContent)) != 0) {
return -1;
}
offsetToContent = ntohl(offsetToContent);
@@ -368,12 +474,14 @@ int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
}
/* Skip to the start of the signature block */
- if (fseeko(fp, SIGNATURE_BLOCK_OFFSET, SEEK_SET)) {
+ *mar_position = 0;
+ if (mar_buffer_seek(mar, mar_position, SIGNATURE_BLOCK_OFFSET) != 0) {
return -1;
}
/* Get the number of signatures */
- if (fread(&signatureCount, sizeof(signatureCount), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &signatureCount, mar_position,
+ sizeof(signatureCount)) != 0) {
return -1;
}
signatureCount = ntohl(signatureCount);
@@ -387,38 +495,50 @@ int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
/* Skip past the whole signature block */
for (i = 0; i < signatureCount; i++) {
/* Skip past the signature algorithm ID */
- if (fseek(fp, sizeof(uint32_t), SEEK_CUR)) {
+ if (mar_buffer_seek(mar, mar_position, sizeof(uint32_t)) != 0) {
return -1;
}
/* Read the signature length and skip past the signature */
- if (fread(&signatureLen, sizeof(uint32_t), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &signatureLen, mar_position,
+ sizeof(uint32_t)) != 0) {
return -1;
}
signatureLen = ntohl(signatureLen);
- if (fseek(fp, signatureLen, SEEK_CUR)) {
+ if (mar_buffer_seek(mar, mar_position, signatureLen) != 0) {
return -1;
}
}
- if ((int64_t)ftell(fp) == (int64_t)offsetToContent) {
+ if (*mar_position <= (size_t)INT64_MAX &&
+ (int64_t)mar_position == (int64_t)offsetToContent) {
*hasAdditionalBlocks = 0;
} else {
if (numAdditionalBlocks) {
/* We have an additional block, so read in the number of additional blocks
and set the offset. */
*hasAdditionalBlocks = 1;
- if (fread(numAdditionalBlocks, sizeof(uint32_t), 1, fp) != 1) {
+ if (mar_read_buffer(mar, numAdditionalBlocks, mar_position,
+ sizeof(uint32_t)) != 0) {
return -1;
}
*numAdditionalBlocks = ntohl(*numAdditionalBlocks);
if (offsetAdditionalBlocks) {
- *offsetAdditionalBlocks = ftell(fp);
+ if (*mar_position > (size_t)UINT32_MAX) {
+ return -1;
+ }
+ *offsetAdditionalBlocks = (uint32_t)*mar_position;
}
} else if (offsetAdditionalBlocks) {
/* numAdditionalBlocks is not specified but offsetAdditionalBlocks
is, so fill it! */
- *offsetAdditionalBlocks = ftell(fp) + sizeof(uint32_t);
+ if (mar_buffer_seek(mar, mar_position, sizeof(uint32_t)) != 0) {
+ return -1;
+ }
+ if (*mar_position > (size_t)UINT32_MAX) {
+ return -1;
+ }
+ *offsetAdditionalBlocks = (uint32_t)*mar_position;
}
}
@@ -436,16 +556,15 @@ int get_mar_file_info_fp(FILE* fp, int* hasSignatureBlock,
int read_product_info_block(char* path,
struct ProductInformationBlock* infoBlock) {
int rv;
- MarFile mar;
- mar.fp = fopen(path, "rb");
- if (!mar.fp) {
+ MarFile* mar;
+ MarReadResult result = mar_open(path, &mar);
+ if (result != MAR_READ_SUCCESS) {
fprintf(stderr,
"ERROR: could not open file in read_product_info_block()\n");
- perror(path);
return -1;
}
- rv = mar_read_product_info_block(&mar, infoBlock);
- fclose(mar.fp);
+ rv = mar_read_product_info_block(mar, infoBlock);
+ mar_close(mar);
return rv;
}
@@ -462,13 +581,14 @@ int mar_read_product_info_block(MarFile* mar,
uint32_t offsetAdditionalBlocks, numAdditionalBlocks, additionalBlockSize,
additionalBlockID;
int hasAdditionalBlocks;
+ size_t mar_position = 0;
/* The buffer size is 97 bytes because the MAR channel name < 64 bytes, and
product version < 32 bytes + 3 NULL terminator bytes. */
char buf[MAXADDITIONALBLOCKSIZE + 1] = {'\0'};
- if (get_mar_file_info_fp(mar->fp, NULL, NULL, &hasAdditionalBlocks,
- &offsetAdditionalBlocks,
- &numAdditionalBlocks) != 0) {
+ if (get_open_mar_file_info(mar, &mar_position, NULL, NULL, &hasAdditionalBlocks,
+ &offsetAdditionalBlocks,
+ &numAdditionalBlocks) != 0) {
return -1;
}
@@ -476,8 +596,8 @@ int mar_read_product_info_block(MarFile* mar,
in a MAR file so check if any exist and process the first found */
if (numAdditionalBlocks > 0) {
/* Read the additional block size */
- if (fread(&additionalBlockSize, sizeof(additionalBlockSize), 1, mar->fp) !=
- 1) {
+ if (mar_read_buffer(mar, &additionalBlockSize, &mar_position,
+ sizeof(additionalBlockSize)) != 0) {
return -1;
}
additionalBlockSize = ntohl(additionalBlockSize) -
@@ -490,7 +610,8 @@ int mar_read_product_info_block(MarFile* mar,
}
/* Read the additional block ID */
- if (fread(&additionalBlockID, sizeof(additionalBlockID), 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, &additionalBlockID, &mar_position,
+ sizeof(additionalBlockID)) != 0) {
return -1;
}
additionalBlockID = ntohl(additionalBlockID);
@@ -499,7 +620,7 @@ int mar_read_product_info_block(MarFile* mar,
const char* location;
int len;
- if (fread(buf, additionalBlockSize, 1, mar->fp) != 1) {
+ if (mar_read_buffer(mar, buf, &mar_position, additionalBlockSize) != 0) {
return -1;
}
@@ -528,7 +649,7 @@ int mar_read_product_info_block(MarFile* mar,
return 0;
} else {
/* This is not the additional block you're looking for. Move along. */
- if (fseek(mar->fp, additionalBlockSize, SEEK_CUR)) {
+ if (mar_buffer_seek(mar, &mar_position, additionalBlockSize) != 0) {
return -1;
}
}
@@ -601,6 +722,7 @@ int mar_enum_items(MarFile* mar, MarItemCallback callback, void* closure) {
int mar_read(MarFile* mar, const MarItem* item, int offset, uint8_t* buf,
int bufsize) {
int nr;
+ size_t mar_position = 0;
if (offset == (int)item->length) {
return 0;
@@ -613,12 +735,16 @@ int mar_read(MarFile* mar, const MarItem* item, int offset, uint8_t* buf,
if (nr > bufsize) {
nr = bufsize;
}
-
- if (fseek(mar->fp, item->offset + offset, SEEK_SET)) {
+
+ // Avoid adding item->offset and offset directly, just in case of overflow.
+ if (mar_buffer_seek(mar, &mar_position, item->offset)) {
+ return -1;
+ }
+ if (mar_buffer_seek(mar, &mar_position, offset)) {
return -1;
}
- return fread(buf, 1, nr, mar->fp);
+ return mar_read_buffer_max(mar, buf, &mar_position, nr);
}
/**
@@ -644,17 +770,18 @@ int get_mar_file_info(const char* path, int* hasSignatureBlock,
uint32_t* offsetAdditionalBlocks,
uint32_t* numAdditionalBlocks) {
int rv;
- FILE* fp = fopen(path, "rb");
- if (!fp) {
- fprintf(stderr, "ERROR: could not open file in get_mar_file_info()\n");
- perror(path);
+ MarFile* mar;
+ size_t mar_position = 0;
+ MarReadResult result = mar_open(path, &mar);
+ if (result != MAR_READ_SUCCESS) {
+ fprintf(stderr, "ERROR: could not read file in get_mar_file_info()\n");
return -1;
}
- rv = get_mar_file_info_fp(fp, hasSignatureBlock, numSignatures,
- hasAdditionalBlocks, offsetAdditionalBlocks,
- numAdditionalBlocks);
+ rv = get_open_mar_file_info(mar, &mar_position, hasSignatureBlock,
+ numSignatures, hasAdditionalBlocks,
+ offsetAdditionalBlocks, numAdditionalBlocks);
- fclose(fp);
+ mar_close(mar);
return rv;
}
diff --git a/modules/libmar/tool/mar.c b/modules/libmar/tool/mar.c
index 0bf2cb4bd1..449c9f7efc 100644
--- a/modules/libmar/tool/mar.c
+++ b/modules/libmar/tool/mar.c
@@ -111,8 +111,8 @@ static int mar_test_callback(MarFile* mar, const MarItem* item, void* unused) {
static int mar_test(const char* path) {
MarFile* mar;
- mar = mar_open(path);
- if (!mar) {
+ MarReadResult result = mar_open(path, &mar);
+ if (result != MAR_READ_SUCCESS) {
return -1;
}
@@ -393,8 +393,9 @@ int main(int argc, char** argv) {
}
if (!rv) {
- MarFile* mar = mar_open(argv[2]);
- if (mar) {
+ MarFile* mar;
+ MarReadResult result = mar_open(argv[2], &mar);
+ if (result == MAR_READ_SUCCESS) {
rv = mar_verify_signatures(mar, certBuffers, fileSizes, certCount);
mar_close(mar);
} else {
diff --git a/modules/libmar/verify/mar_verify.c b/modules/libmar/verify/mar_verify.c
index 2ec17bbf7f..f08721c323 100644
--- a/modules/libmar/verify/mar_verify.c
+++ b/modules/libmar/verify/mar_verify.c
@@ -56,35 +56,38 @@ int mar_read_entire_file(const char* filePath, uint32_t maxSize,
return result;
}
-int mar_extract_and_verify_signatures_fp(FILE* fp,
- CryptoX_ProviderHandle provider,
- CryptoX_PublicKey* keys,
- uint32_t keyCount);
-int mar_verify_signatures_for_fp(FILE* fp, CryptoX_ProviderHandle provider,
- CryptoX_PublicKey* keys,
- const uint8_t* const* extractedSignatures,
- uint32_t keyCount, uint32_t* numVerified);
+int mar_extract_and_verify_signatures(MarFile* mar,
+ CryptoX_ProviderHandle provider,
+ CryptoX_PublicKey* keys,
+ uint32_t keyCount);
+int mar_verify_extracted_signatures(MarFile* mar,
+ CryptoX_ProviderHandle provider,
+ CryptoX_PublicKey* keys,
+ const uint8_t* const* extractedSignatures,
+ uint32_t keyCount, uint32_t* numVerified);
/**
- * Reads the specified number of bytes from the file pointer and
+ * Reads the specified number of bytes from the MAR buffer and
* stores them in the passed buffer.
*
- * @param fp The file pointer to read from.
+ * @param mar An opened MAR
+ * @param mar_position
+ * Our current position within the MAR file buffer.
* @param buffer The buffer to store the read results.
* @param size The number of bytes to read, buffer must be
* at least of this size.
* @param ctxs Pointer to the first element in an array of verify context.
* @param count The number of elements in ctxs
* @param err The name of what is being written to in case of error.
- * @return 0 on success
- * -1 on read error
- * -2 on verify update error
+ * @return CryptoX_Success on success
+ * CryptoX_Error on error
*/
-int ReadAndUpdateVerifyContext(FILE* fp, void* buffer, uint32_t size,
- CryptoX_SignatureHandle* ctxs, uint32_t count,
- const char* err) {
+CryptoX_Result ReadAndUpdateVerifyContext(MarFile* mar, size_t* mar_position,
+ void* buffer, uint32_t size,
+ CryptoX_SignatureHandle* ctxs,
+ uint32_t count, const char* err) {
uint32_t k;
- if (!fp || !buffer || !ctxs || count == 0 || !err) {
+ if (!mar || !mar_position || !buffer || !ctxs || count == 0 || !err) {
fprintf(stderr, "ERROR: Invalid parameter specified.\n");
return CryptoX_Error;
}
@@ -93,7 +96,7 @@ int ReadAndUpdateVerifyContext(FILE* fp, void* buffer, uint32_t size,
return CryptoX_Success;
}
- if (fread(buffer, size, 1, fp) != 1) {
+ if (mar_read_buffer(mar, buffer, mar_position, size) != 0) {
fprintf(stderr, "ERROR: Could not read %s\n", err);
return CryptoX_Error;
}
@@ -101,7 +104,7 @@ int ReadAndUpdateVerifyContext(FILE* fp, void* buffer, uint32_t size,
for (k = 0; k < count; k++) {
if (CryptoX_Failed(CryptoX_VerifyUpdate(&ctxs[k], buffer, size))) {
fprintf(stderr, "ERROR: Could not update verify context for %s\n", err);
- return -2;
+ return CryptoX_Error;
}
}
return CryptoX_Success;
@@ -136,11 +139,6 @@ int mar_verify_signatures(MarFile* mar, const uint8_t* const* certData,
goto failure;
}
- if (!mar->fp) {
- fprintf(stderr, "ERROR: MAR file is not open.\n");
- goto failure;
- }
-
if (CryptoX_Failed(CryptoX_InitCryptoProvider(&provider))) {
fprintf(stderr, "ERROR: Could not init crytpo library.\n");
goto failure;
@@ -154,7 +152,7 @@ int mar_verify_signatures(MarFile* mar, const uint8_t* const* certData,
}
}
- rv = mar_extract_and_verify_signatures_fp(mar->fp, provider, keys, certCount);
+ rv = mar_extract_and_verify_signatures(mar, provider, keys, certCount);
failure:
@@ -169,50 +167,41 @@ failure:
/**
* Extracts each signature from the specified MAR file,
- * then calls mar_verify_signatures_for_fp to verify each signature.
+ * then calls mar_verify_extracted_signatures to verify each signature.
*
- * @param fp An opened MAR file handle
+ * @param mar An opened MAR
* @param provider A library provider
* @param keys The public keys to use to verify the MAR
* @param keyCount The number of keys pointed to by keys
* @return 0 on success
*/
-int mar_extract_and_verify_signatures_fp(FILE* fp,
- CryptoX_ProviderHandle provider,
- CryptoX_PublicKey* keys,
- uint32_t keyCount) {
+int mar_extract_and_verify_signatures(MarFile* mar,
+ CryptoX_ProviderHandle provider,
+ CryptoX_PublicKey* keys,
+ uint32_t keyCount) {
uint32_t signatureCount, signatureLen, numVerified = 0;
uint32_t signatureAlgorithmIDs[MAX_SIGNATURES];
uint8_t* extractedSignatures[MAX_SIGNATURES];
uint32_t i;
+ size_t mar_position = 0;
memset(signatureAlgorithmIDs, 0, sizeof(signatureAlgorithmIDs));
memset(extractedSignatures, 0, sizeof(extractedSignatures));
- if (!fp) {
+ if (!mar) {
fprintf(stderr, "ERROR: Invalid file pointer passed.\n");
return CryptoX_Error;
}
- /* To protect against invalid MAR files, we assumes that the MAR file
- size is less than or equal to MAX_SIZE_OF_MAR_FILE. */
- if (fseeko(fp, 0, SEEK_END)) {
- fprintf(stderr, "ERROR: Could not seek to the end of the MAR file.\n");
- return CryptoX_Error;
- }
- if (ftello(fp) > MAX_SIZE_OF_MAR_FILE) {
- fprintf(stderr, "ERROR: MAR file is too large to be verified.\n");
- return CryptoX_Error;
- }
-
/* Skip to the start of the signature block */
- if (fseeko(fp, SIGNATURE_BLOCK_OFFSET, SEEK_SET)) {
+ if (mar_buffer_seek(mar, &mar_position, SIGNATURE_BLOCK_OFFSET) != 0) {
fprintf(stderr, "ERROR: Could not seek to the signature block.\n");
return CryptoX_Error;
}
/* Get the number of signatures */
- if (fread(&signatureCount, sizeof(signatureCount), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &signatureCount, &mar_position,
+ sizeof(signatureCount)) != 0) {
fprintf(stderr, "ERROR: Could not read number of signatures.\n");
return CryptoX_Error;
}
@@ -228,19 +217,21 @@ int mar_extract_and_verify_signatures_fp(FILE* fp,
for (i = 0; i < signatureCount; i++) {
/* Get the signature algorithm ID */
- if (fread(&signatureAlgorithmIDs[i], sizeof(uint32_t), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &signatureAlgorithmIDs[i], &mar_position,
+ sizeof(uint32_t)) != 0) {
fprintf(stderr, "ERROR: Could not read signatures algorithm ID.\n");
return CryptoX_Error;
}
signatureAlgorithmIDs[i] = ntohl(signatureAlgorithmIDs[i]);
- if (fread(&signatureLen, sizeof(uint32_t), 1, fp) != 1) {
+ if (mar_read_buffer(mar, &signatureLen, &mar_position,
+ sizeof(uint32_t)) != 0) {
fprintf(stderr, "ERROR: Could not read signatures length.\n");
return CryptoX_Error;
}
signatureLen = ntohl(signatureLen);
- /* To protected against invalid input make sure the signature length
+ /* To protect against invalid input make sure the signature length
isn't too big. */
if (signatureLen > MAX_SIGNATURE_LENGTH) {
fprintf(stderr, "ERROR: Signature length is too large to verify.\n");
@@ -249,10 +240,11 @@ int mar_extract_and_verify_signatures_fp(FILE* fp,
extractedSignatures[i] = malloc(signatureLen);
if (!extractedSignatures[i]) {
- fprintf(stderr, "ERROR: Could allocate buffer for signature.\n");
+ fprintf(stderr, "ERROR: Could not allocate buffer for signature.\n");
return CryptoX_Error;
}
- if (fread(extractedSignatures[i], signatureLen, 1, fp) != 1) {
+ if (mar_read_buffer(mar, extractedSignatures[i], &mar_position,
+ signatureLen) != 0) {
fprintf(stderr, "ERROR: Could not read extracted signature.\n");
for (i = 0; i < signatureCount; ++i) {
free(extractedSignatures[i]);
@@ -270,11 +262,8 @@ int mar_extract_and_verify_signatures_fp(FILE* fp,
}
}
- if (ftello(fp) == -1) {
- return CryptoX_Error;
- }
- if (mar_verify_signatures_for_fp(
- fp, provider, keys, (const uint8_t* const*)extractedSignatures,
+ if (mar_verify_extracted_signatures(
+ mar, provider, keys, (const uint8_t* const*)extractedSignatures,
signatureCount, &numVerified) == CryptoX_Error) {
return CryptoX_Error;
}
@@ -304,7 +293,7 @@ int mar_extract_and_verify_signatures_fp(FILE* fp,
* certificate given, etc. The signature count must exactly match the number of
* certificates given, and all signature verifications must succeed.
*
- * @param fp An opened MAR file handle
+ * @param mar An opened MAR
* @param provider A library provider
* @param keys A pointer to the first element in an
* array of keys.
@@ -315,18 +304,19 @@ int mar_extract_and_verify_signatures_fp(FILE* fp,
* the number of verified signatures.
* This information can be useful for printing
* error messages.
- * @return 0 on success, *numVerified == signatureCount.
+ * @return CryptoX_Success on success, *numVerified == signatureCount.
*/
-int mar_verify_signatures_for_fp(FILE* fp, CryptoX_ProviderHandle provider,
- CryptoX_PublicKey* keys,
- const uint8_t* const* extractedSignatures,
- uint32_t signatureCount,
- uint32_t* numVerified) {
+CryptoX_Result mar_verify_extracted_signatures(
+ MarFile* mar, CryptoX_ProviderHandle provider, CryptoX_PublicKey* keys,
+ const uint8_t* const* extractedSignatures,
+ uint32_t signatureCount,
+ uint32_t* numVerified) {
CryptoX_SignatureHandle signatureHandles[MAX_SIGNATURES];
char buf[BLOCKSIZE];
uint32_t signatureLengths[MAX_SIGNATURES];
uint32_t i;
int rv = CryptoX_Error;
+ size_t mar_position = 0;
memset(signatureHandles, 0, sizeof(signatureHandles));
memset(signatureLengths, 0, sizeof(signatureLengths));
@@ -355,19 +345,13 @@ int mar_verify_signatures_for_fp(FILE* fp, CryptoX_ProviderHandle provider,
}
}
- /* Skip to the start of the file */
- if (fseeko(fp, 0, SEEK_SET)) {
- fprintf(stderr, "ERROR: Could not seek to start of the file\n");
- goto failure;
- }
-
/* Bytes 0-3: MAR1
Bytes 4-7: index offset
Bytes 8-15: size of entire MAR
*/
if (CryptoX_Failed(ReadAndUpdateVerifyContext(
- fp, buf, SIGNATURE_BLOCK_OFFSET + sizeof(uint32_t), signatureHandles,
- signatureCount, "signature block"))) {
+ mar, &mar_position, buf, SIGNATURE_BLOCK_OFFSET + sizeof(uint32_t),
+ signatureHandles, signatureCount, "signature block"))) {
goto failure;
}
@@ -375,14 +359,14 @@ int mar_verify_signatures_for_fp(FILE* fp, CryptoX_ProviderHandle provider,
for (i = 0; i < signatureCount; i++) {
/* Get the signature algorithm ID */
if (CryptoX_Failed(ReadAndUpdateVerifyContext(
- fp, &buf, sizeof(uint32_t), signatureHandles, signatureCount,
- "signature algorithm ID"))) {
+ mar, &mar_position, &buf, sizeof(uint32_t), signatureHandles,
+ signatureCount, "signature algorithm ID"))) {
goto failure;
}
if (CryptoX_Failed(ReadAndUpdateVerifyContext(
- fp, &signatureLengths[i], sizeof(uint32_t), signatureHandles,
- signatureCount, "signature length"))) {
+ mar, &mar_position, &signatureLengths[i], sizeof(uint32_t),
+ signatureHandles, signatureCount, "signature length"))) {
goto failure;
}
signatureLengths[i] = ntohl(signatureLengths[i]);
@@ -392,20 +376,15 @@ int mar_verify_signatures_for_fp(FILE* fp, CryptoX_ProviderHandle provider,
}
/* Skip past the signature itself as those are not included */
- if (fseeko(fp, signatureLengths[i], SEEK_CUR)) {
+ if (mar_buffer_seek(mar, &mar_position, signatureLengths[i]) != 0) {
fprintf(stderr, "ERROR: Could not seek past signature.\n");
goto failure;
}
}
/* Read the rest of the file after the signature block */
- while (!feof(fp)) {
- int numRead = fread(buf, 1, BLOCKSIZE, fp);
- if (ferror(fp)) {
- fprintf(stderr, "ERROR: Error reading data block.\n");
- goto failure;
- }
-
+ while (mar_position < mar->data_len) {
+ int numRead = mar_read_buffer_max(mar, buf, &mar_position, BLOCKSIZE);
for (i = 0; i < signatureCount; i++) {
if (CryptoX_Failed(
CryptoX_VerifyUpdate(&signatureHandles[i], buf, numRead))) {
diff --git a/toolkit/mozapps/update/UpdateService.jsm b/toolkit/mozapps/update/UpdateService.jsm
index 4d1b1c59ef..e7cb58d0a0 100644
--- a/toolkit/mozapps/update/UpdateService.jsm
+++ b/toolkit/mozapps/update/UpdateService.jsm
@@ -1943,6 +1943,22 @@ function updateIsAtLeastAsOldAs(update, version, buildID) {
);
}
+/**
+ * This function determines whether the error represented by the passed error
+ * code is the result of the updater failing to allocate memory. This is
+ * relevant when staging because, since Firefox is also running, we may not be
+ * able to allocate much memory. Thus, if we fail to stage an update, we may
+ * succeed at updating without staging.
+ *
+ * @param An integer error code from the update.status file. Should be one of
+ * the codes enumerated in updatererrors.h.
+ * @returns true if the code represents a memory allocation error.
+ * Otherwise, false.
+ */
+function isMemoryAllocationErrorCode(errorCode) {
+ return errorCode >= 10 && errorCode <= 14;
+}
+
/**
* Update Patch
* @param patch
@@ -4485,12 +4501,26 @@ UpdateManager.prototype = {
cleanUpReadyUpdateDir(false);
if (update.state == STATE_FAILED && parts[1]) {
+ let isMemError = isMemoryAllocationErrorCode(parts[1]);
if (
parts[1] == DELETE_ERROR_STAGING_LOCK_FILE ||
- parts[1] == UNEXPECTED_STAGING_ERROR
+ parts[1] == UNEXPECTED_STAGING_ERROR ||
+ isMemError
) {
update.state = getBestPendingState();
writeStatusFile(getReadyUpdateDir(), update.state);
+ if (isMemError) {
+ LOG(
+ `UpdateManager:refreshUpdateStatus - Updater failed to ` +
+ `allocate enough memory to successfully stage. Setting ` +
+ `status to "${update.state}"`
+ );
+ } else {
+ LOG(
+ `UpdateManager:refreshUpdateStatus - Unexpected staging error. ` +
+ `Setting status to "${update.state}"`
+ );
+ }
} else if (!handleUpdateFailure(update, parts[1])) {
await handleFallbackToCompleteUpdate(true);
}
diff --git a/toolkit/mozapps/update/common/updatererrors.h b/toolkit/mozapps/update/common/updatererrors.h
index c4388cf3fd..a5304fceb3 100644
--- a/toolkit/mozapps/update/common/updatererrors.h
+++ b/toolkit/mozapps/update/common/updatererrors.h
@@ -23,11 +23,17 @@
#define WRITE_ERROR 7
// #define UNEXPECTED_ERROR 8 // Replaced with errors 38-42
#define ELEVATION_CANCELED 9
+
+// Error codes 10-14 are related to memory allocation failures.
+// Note: If more memory allocation error codes are added, the implementation of
+// isMemoryAllocationErrorCode in UpdateService.jsm should be updated to account
+// for them.
#define READ_STRINGS_MEM_ERROR 10
#define ARCHIVE_READER_MEM_ERROR 11
#define BSPATCH_MEM_ERROR 12
#define UPDATER_MEM_ERROR 13
#define UPDATER_QUOTED_PATH_MEM_ERROR 14
+
#define BAD_ACTION_ERROR 15
#define STRING_CONVERSION_ERROR 16
diff --git a/toolkit/mozapps/update/tests/browser/browser.ini b/toolkit/mozapps/update/tests/browser/browser.ini
index edb9e2260e..00935bca0d 100644
--- a/toolkit/mozapps/update/tests/browser/browser.ini
+++ b/toolkit/mozapps/update/tests/browser/browser.ini
@@ -99,3 +99,6 @@ reason = test must be able to prevent file deletion.
# Telemetry Update Ping Tests
[browser_telemetry_updatePing_downloaded_ready.js]
[browser_telemetry_updatePing_staged_ready.js]
+
+# Memory Fallback Tests
+[browser_memory_allocation_error_fallback.js]
diff --git a/toolkit/mozapps/update/tests/browser/browser_memory_allocation_error_fallback.js b/toolkit/mozapps/update/tests/browser/browser_memory_allocation_error_fallback.js
new file mode 100644
index 0000000000..1bc075ef5e
--- /dev/null
+++ b/toolkit/mozapps/update/tests/browser/browser_memory_allocation_error_fallback.js
@@ -0,0 +1,81 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+/**
+ * When the updater fails with a memory allocation error, we should fall back to
+ * updating without staging.
+ */
+
+const READ_STRINGS_MEM_ERROR = 10;
+const ARCHIVE_READER_MEM_ERROR = 11;
+const BSPATCH_MEM_ERROR = 12;
+const UPDATER_MEM_ERROR = 13;
+const UPDATER_QUOTED_PATH_MEM_ERROR = 14;
+
+const EXPECTED_STATUS =
+ AppConstants.platform == "win" ? STATE_PENDING_SVC : STATE_PENDING;
+
+add_setup(async function setup() {
+ await SpecialPowers.pushPrefEnv({
+ set: [
+ [PREF_APP_UPDATE_STAGING_ENABLED, true],
+ [PREF_APP_UPDATE_SERVICE_ENABLED, true],
+ ],
+ });
+
+ registerCleanupFunction(() => {
+ Services.env.set("MOZ_FORCE_ERROR_CODE", "");
+ });
+});
+
+async function memAllocErrorFallback(errorCode) {
+ Services.env.set("MOZ_FORCE_ERROR_CODE", errorCode.toString());
+
+ // Since the partial should be successful specify an invalid size for the
+ // complete update.
+ let params = {
+ queryString: "&invalidCompleteSize=1",
+ backgroundUpdate: true,
+ continueFile: CONTINUE_STAGING,
+ waitForUpdateState: EXPECTED_STATUS,
+ };
+ await runAboutPrefsUpdateTest(params, [
+ {
+ panelId: "apply",
+ checkActiveUpdate: { state: EXPECTED_STATUS },
+ continueFile: null,
+ },
+ ]);
+}
+
+function cleanup() {
+ reloadUpdateManagerData(true);
+ removeUpdateFiles(true);
+}
+
+add_task(async function memAllocErrorFallback_READ_STRINGS_MEM_ERROR() {
+ await memAllocErrorFallback(READ_STRINGS_MEM_ERROR);
+ cleanup();
+});
+
+add_task(async function memAllocErrorFallback_ARCHIVE_READER_MEM_ERROR() {
+ await memAllocErrorFallback(ARCHIVE_READER_MEM_ERROR);
+ cleanup();
+});
+
+add_task(async function memAllocErrorFallback_BSPATCH_MEM_ERROR() {
+ await memAllocErrorFallback(BSPATCH_MEM_ERROR);
+ cleanup();
+});
+
+add_task(async function memAllocErrorFallback_UPDATER_MEM_ERROR() {
+ await memAllocErrorFallback(UPDATER_MEM_ERROR);
+ cleanup();
+});
+
+add_task(async function memAllocErrorFallback_UPDATER_QUOTED_PATH_MEM_ERROR() {
+ await memAllocErrorFallback(UPDATER_QUOTED_PATH_MEM_ERROR);
+ cleanup();
+});
diff --git a/toolkit/mozapps/update/updater/archivereader.cpp b/toolkit/mozapps/update/updater/archivereader.cpp
index e166310b13..f73c24bd98 100644
--- a/toolkit/mozapps/update/updater/archivereader.cpp
+++ b/toolkit/mozapps/update/updater/archivereader.cpp
@@ -205,12 +205,15 @@ int ArchiveReader::Open(const NS_tchar* path) {
}
}
+MarReadResult result =
#ifdef XP_WIN
- mArchive = mar_wopen(path);
+ mar_wopen(path, &mArchive);
#else
- mArchive = mar_open(path);
+ mar_open(path, &mArchive);
#endif
- if (!mArchive) {
+ if (result == MAR_MEM_ERROR) {
+ return ARCHIVE_READER_MEM_ERROR;
+ } else if (result != MAR_READ_SUCCESS) {
return READ_ERROR;
}
diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp
index 0295d2435d..2b0a94a6d1 100644
--- a/toolkit/mozapps/update/updater/updater.cpp
+++ b/toolkit/mozapps/update/updater/updater.cpp
@@ -2586,6 +2586,12 @@ static void UpdateThreadFunc(void* param) {
putenv(const_cast<char*>("MOZ_TEST_PROCESS_UPDATES="));
#endif
} else {
+#ifdef TEST_UPDATER
+ const char* forceErrorCodeString = getenv("MOZ_FORCE_ERROR_CODE");
+ if (forceErrorCodeString && *forceErrorCodeString) {
+ rv = atoi(forceErrorCodeString);
+ }
+#endif
if (rv) {
LOG(("failed: %d", rv));
} else {
--
2.27.0
Reference in New Issue View Git Blame Copy Permalink