From 64a11edb020de40aba05b3ad70e55133c500edbf Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Sat, 29 Dec 2018 00:11:45 +0100 Subject: [PATCH] app_server: Make more use of BStackOrHeapArray. Variable length arrays on the stack are always risky when the length is indeterminate as they can easily overflow the stack. Replace their use by BStackOrHeapArray, fixes #6354. Also replace most other dynamic allocations by BStackOrHeapArray as it is more convenient and may avoid unnecessary dynamic allocations. Add allocation checks and early returns to all places while at it. --- src/servers/app/ServerApp.cpp | 99 ++++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 43 deletions(-) diff --git a/src/servers/app/ServerApp.cpp b/src/servers/app/ServerApp.cpp index e81d9a5495..7abbbac0b8 100644 --- a/src/servers/app/ServerApp.cpp +++ b/src/servers/app/ServerApp.cpp @@ -1867,6 +1867,13 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) BStackOrHeapArray widthArray(numStrings); BStackOrHeapArray lengthArray(numStrings); BStackOrHeapArray stringArray(numStrings); + if (!widthArray.IsValid() || !lengthArray.IsValid() + || !stringArray.IsValid()) { + fLink.StartMessage(B_NO_MEMORY); + fLink.Flush(); + break; + } + for (int32 i = 0; i < numStrings; i++) { // This version of ReadString allocates the strings, we free // them below @@ -2129,8 +2136,14 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) link.Read(&numChars); link.Read(&numBytes); - // TODO: proper error checking - char* charArray = new (nothrow) char[numBytes]; + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray shapes(numChars); + if (!charArray.IsValid() || !shapes.IsValid()) { + fLink.StartMessage(B_NO_MEMORY); + fLink.Flush(); + break; + } + link.Read(charArray, numBytes); ServerFont font; @@ -2142,8 +2155,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) font.SetFalseBoldWidth(falseBoldWidth); font.SetFlags(flags); - // TODO: proper error checking - BShape** shapes = new (nothrow) BShape*[numChars]; status = font.GetGlyphShapes(charArray, numChars, shapes); if (status == B_OK) { fLink.StartMessage(B_OK); @@ -2154,11 +2165,9 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(status); - delete[] shapes; } else fLink.StartMessage(status); - delete[] charArray; fLink.Flush(); break; } @@ -2181,24 +2190,29 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) int32 numChars, numBytes; link.Read(&numChars); link.Read(&numBytes); - // TODO: proper error checking - char* charArray = new (nothrow) char[numBytes]; + + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray hasArray(numChars); + if (!charArray.IsValid() || !hasArray.IsValid()) { + fLink.StartMessage(B_NO_MEMORY); + fLink.Flush(); + break; + } + link.Read(charArray, numBytes); ServerFont font; status_t status = font.SetFamilyAndStyle(familyID, styleID); if (status == B_OK) { - bool hasArray[numChars]; status = font.GetHasGlyphs(charArray, numBytes, hasArray); if (status == B_OK) { fLink.StartMessage(B_OK); - fLink.Attach(hasArray, sizeof(hasArray)); + fLink.Attach(hasArray, numChars * sizeof(bool)); } else fLink.StartMessage(status); } else fLink.StartMessage(status); - delete[] charArray; fLink.Flush(); break; } @@ -2223,24 +2237,29 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) uint32 numBytes; link.Read(&numBytes); - // TODO: proper error checking - char* charArray = new (nothrow) char[numBytes]; + + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray edgeArray(numChars); + if (!charArray.IsValid() || !edgeArray.IsValid()) { + fLink.StartMessage(B_NO_MEMORY); + fLink.Flush(); + break; + } + link.Read(charArray, numBytes); ServerFont font; status_t status = font.SetFamilyAndStyle(familyID, styleID); if (status == B_OK) { - edge_info edgeArray[numChars]; status = font.GetEdges(charArray, numBytes, edgeArray); if (status == B_OK) { fLink.StartMessage(B_OK); - fLink.Attach(edgeArray, sizeof(edgeArray)); + fLink.Attach(edgeArray, numChars * sizeof(edge_info)); } else fLink.StartMessage(status); } else fLink.StartMessage(status); - delete[] charArray; fLink.Flush(); break; } @@ -2289,16 +2308,14 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) uint32 numBytes; link.Read(&numBytes); - char* charArray = new(std::nothrow) char[numBytes]; - BPoint* escapements = new(std::nothrow) BPoint[numChars]; + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray escapements(numChars); BPoint* offsets = NULL; if (wantsOffsets) offsets = new(std::nothrow) BPoint[numChars]; - if (charArray == NULL || escapements == NULL + if (!charArray.IsValid() || !escapements.IsValid() || (offsets == NULL && wantsOffsets)) { - delete[] charArray; - delete[] escapements; delete[] offsets; fLink.StartMessage(B_NO_MEMORY); fLink.Flush(); @@ -2332,8 +2349,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } else fLink.StartMessage(status); - delete[] charArray; - delete[] escapements; delete[] offsets; fLink.Flush(); break; @@ -2381,11 +2396,9 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) uint32 numBytes; link.Read(&numBytes); - char* charArray = new (nothrow) char[numBytes]; - float* escapements = new (nothrow) float[numChars]; - if (charArray == NULL || escapements == NULL) { - delete[] charArray; - delete[] escapements; + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray escapements(numChars); + if (!charArray.IsValid() || !escapements.IsValid()) { fLink.StartMessage(B_NO_MEMORY); fLink.Flush(); break; @@ -2412,9 +2425,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) } } - delete[] charArray; - delete[] escapements; - if (status != B_OK) fLink.StartMessage(status); @@ -2475,9 +2485,9 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) bool success = false; - char* charArray = new(std::nothrow) char[numBytes]; - BRect* rectArray = new(std::nothrow) BRect[numChars]; - if (charArray != NULL && rectArray != NULL) { + BStackOrHeapArray charArray(numBytes); + BStackOrHeapArray rectArray(numChars); + if (charArray.IsValid() && rectArray.IsValid()) { link.Read(charArray, numBytes); // figure out escapements @@ -2509,9 +2519,6 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) fLink.StartMessage(B_ERROR); fLink.Flush(); - - delete[] charArray; - delete[] rectArray; break; } @@ -2557,18 +2564,24 @@ ServerApp::_DispatchMessage(int32 code, BPrivate::LinkReceiver& link) int32 numStrings; link.Read(&numStrings); - escapement_delta deltaArray[numStrings]; - char* stringArray[numStrings]; - int32 lengthArray[numStrings]; - for(int32 i = 0; i < numStrings; i++) { + BStackOrHeapArray deltaArray(numStrings); + BStackOrHeapArray stringArray(numStrings); + BStackOrHeapArray lengthArray(numStrings); + BStackOrHeapArray rectArray(numStrings); + if (!deltaArray.IsValid() || !stringArray.IsValid() + || !lengthArray.IsValid() || !rectArray.IsValid()) { + fLink.StartMessage(B_NO_MEMORY); + fLink.Flush(); + break; + } + + for (int32 i = 0; i < numStrings; i++) { // This version of ReadString allocates the strings, we free // them below link.ReadString(&stringArray[i], &lengthArray[i]); link.Read(&deltaArray[i]); } - BStackOrHeapArray rectArray(numStrings); - ServerFont font; bool success = false; if (font.SetFamilyAndStyle(familyID, styleID) == B_OK) {