Bug fixes to 1.19 alpha

Fixed buffer overflow in OutputBuffer::cat when the input string is
longer than the remaining size in the buffer plus one new buffer
Fixed issue with sending output buffers totlaling more then 2Kb to the
Ethernet interface on the Duet Ethernet
This commit is contained in:
David Crocker 2017-04-21 15:14:50 +01:00
parent 7fea2926dc
commit 636b3fc378
6 changed files with 55 additions and 105 deletions

View file

@ -78,7 +78,7 @@ void NetworkResponder::SendData()
}
else
{
const size_t sent = skt->Send(reinterpret_cast<const uint8_t *>(outBuf->Data()), bytesLeft);
const size_t sent = skt->Send(reinterpret_cast<const uint8_t *>(outBuf->UnreadData()), bytesLeft);
if (sent == 0)
{
// Check whether the connection has been closed

View file

@ -286,16 +286,13 @@ size_t Socket::Send(const uint8_t *data, size_t length)
}
wiz_send_data_at(socketNum, data, length, wizTxBufferPtr);
wizTxBufferLeft -= length;
wizTxBufferPtr += length;
if (wizTxBufferLeft == 0)
{
// Buffer is full so send it
ExecCommand(socketNum, Sn_CR_SEND);
isSending = true;
sendOutstanding = false;
Send();
}
else
{
wizTxBufferPtr += length;
sendOutstanding = true;
}
return length;
@ -307,6 +304,7 @@ void Socket::Send()
{
if (CanSend() && sendOutstanding)
{
setSn_TX_WR(socketNum, wizTxBufferPtr);
ExecCommand(socketNum, Sn_CR_SEND);
isSending = true;
sendOutstanding = false;

View file

@ -150,9 +150,6 @@ void wiz_send_data_at(uint8_t sn, const uint8_t *wizdata, uint16_t len, uint16_t
{
const uint32_t addrsel = ((uint32_t)ptr << 8) + (WIZCHIP_TXBUF_BLOCK(sn) << 3);
WIZCHIP_WRITE_BUF(addrsel, wizdata, len);
ptr += len;
setSn_TX_WR(sn,ptr);
}
}

View file

@ -114,6 +114,7 @@ size_t OutputBuffer::catf(const char *fmt, ...)
vsnprintf(formatBuffer, ARRAY_SIZE(formatBuffer), fmt, vargs);
va_end(vargs);
formatBuffer[ARRAY_UPB(formatBuffer)] = 0;
return cat(formatBuffer);
}
@ -148,54 +149,8 @@ size_t OutputBuffer::copy(const char *src, size_t len)
last = this;
}
// Does the whole string fit into this instance?
if (len > OUTPUT_BUFFER_SIZE)
{
// No - copy what we can't write into a new chain
OutputBuffer *currentBuffer;
size_t bytesCopied = OUTPUT_BUFFER_SIZE;
do {
if (!Allocate(currentBuffer))
{
// We cannot store the whole string, stop here
break;
}
currentBuffer->references = references;
// Fill up the next instance
const size_t copyLength = min<size_t>(OUTPUT_BUFFER_SIZE, len - bytesCopied);
memcpy(currentBuffer->data, src + bytesCopied, copyLength);
currentBuffer->dataLength = copyLength;
bytesCopied += copyLength;
// Link it to the chain
if (next == nullptr)
{
next = last = currentBuffer;
}
else
{
last->next = currentBuffer;
last = currentBuffer;
}
} while (bytesCopied < len);
// Update references to the last entry for all items
for(OutputBuffer *item = Next(); item != last; item = item->Next())
{
item->last = last;
}
// Then copy the rest into this instance
memcpy(data, src, OUTPUT_BUFFER_SIZE);
dataLength = OUTPUT_BUFFER_SIZE;
return bytesCopied;
}
// Yes, the whole string fits into this instance. No need to allocate a new item
memcpy(data, src, len);
dataLength = len;
return len;
dataLength = 0;
return cat(src, len);
}
size_t OutputBuffer::cat(const char c)
@ -235,36 +190,32 @@ size_t OutputBuffer::cat(const char *src)
size_t OutputBuffer::cat(const char *src, size_t len)
{
// Copy what we can into the last buffer
size_t copyLength = min<size_t>(len, OUTPUT_BUFFER_SIZE - last->dataLength);
memcpy(last->data + last->dataLength, src, copyLength);
last->dataLength += copyLength;
// Is there any more data left?
if (len > copyLength)
size_t copied = 0;
while (copied < len)
{
// Yes - copy what we couldn't write into a new chain
OutputBuffer *nextBuffer;
if (!Allocate(nextBuffer))
if (last->dataLength == OUTPUT_BUFFER_SIZE)
{
// We cannot store any more data, stop here
return copyLength;
// The last buffer is full
OutputBuffer *nextBuffer;
if (!Allocate(nextBuffer))
{
// We cannot store any more data, stop here
break;
}
nextBuffer->references = references;
last->next = nextBuffer;
last = nextBuffer->last;
for (OutputBuffer *item = Next(); item != nextBuffer; item = item->Next())
{
item->last = last;
}
}
nextBuffer->references = references;
const size_t bytesCopied = copyLength + nextBuffer->copy(src + copyLength, len - copyLength);
// Done - now append the new entry to the chain
last->next = nextBuffer;
last = nextBuffer->last;
for(OutputBuffer *item = Next(); item != nextBuffer; item = item->Next())
{
item->last = last;
}
return bytesCopied;
const size_t copyLength = min<size_t>(len - copied, OUTPUT_BUFFER_SIZE - last->dataLength);
memcpy(last->data + last->dataLength, src + copied, copyLength);
last->dataLength += copyLength;
copied += copyLength;
}
// No more data had to be written, we could store everything
return len;
return copied;
}
size_t OutputBuffer::cat(StringRef &str)
@ -281,13 +232,15 @@ size_t OutputBuffer::EncodeString(const char *src, size_t srcLength, bool allowC
bytesWritten += cat('"');
}
size_t srcPointer = 1;
char c = *src++;
while (srcPointer <= srcLength && c != 0 && (c >= ' ' || allowControlChars))
if (srcLength != 0)
{
char esc;
switch (c)
size_t srcPointer = 1;
char c = *src++;
while (srcPointer <= srcLength && c != 0 && (c >= ' ' || allowControlChars))
{
char esc;
switch (c)
{
case '\r':
esc = 'r';
break;
@ -310,20 +263,21 @@ size_t OutputBuffer::EncodeString(const char *src, size_t srcLength, bool allowC
default:
esc = 0;
break;
}
}
if (esc != 0)
{
bytesWritten += cat('\\');
bytesWritten += cat(esc);
}
else
{
bytesWritten += cat(c);
}
if (esc != 0)
{
bytesWritten += cat('\\');
bytesWritten += cat(esc);
}
else
{
bytesWritten += cat(c);
}
c = *src++;
srcPointer++;
c = *src++;
srcPointer++;
}
}
if (encapsulateString)
@ -393,7 +347,7 @@ size_t OutputBuffer::EncodeReply(OutputBuffer *src, bool allowControlChars)
// Get the number of bytes left for continuous writing
/*static*/ size_t OutputBuffer::GetBytesLeft(const OutputBuffer *writingBuffer)
{
size_t freeOutputBuffers = OUTPUT_BUFFER_COUNT - usedOutputBuffers;
const size_t freeOutputBuffers = OUTPUT_BUFFER_COUNT - usedOutputBuffers;
if (writingBuffer == nullptr)
{
// Only return the total number of bytes left
@ -401,11 +355,11 @@ size_t OutputBuffer::EncodeReply(OutputBuffer *src, bool allowControlChars)
}
// We're doing a possibly long response like a filelist
size_t bytesLeft = OUTPUT_BUFFER_SIZE - writingBuffer->last->DataLength();
const size_t bytesLeft = OUTPUT_BUFFER_SIZE - writingBuffer->last->DataLength();
if (freeOutputBuffers < RESERVED_OUTPUT_BUFFERS)
{
// Keep some space left to encapsulate the respones (e.g. via an HTTP header)
// Keep some space left to encapsulate the responses (e.g. via an HTTP header)
return bytesLeft;
}

View file

@ -29,6 +29,7 @@ class OutputBuffer
void IncreaseReferences(size_t refs);
const char *Data() const { return data; }
const char *UnreadData() const { return data + bytesRead; }
size_t DataLength() const { return dataLength; } // How many bytes have been written to this instance?
size_t Length() const; // How many bytes have been written to the whole chain?

View file

@ -9,11 +9,11 @@
#define SRC_VERSION_H_
#ifndef VERSION
# define VERSION "1.19alpha1b"
# define VERSION "1.19alpha1"
#endif
#ifndef DATE
# define DATE "2017-04-17"
# define DATE "2017-04-21"
#endif
#define AUTHORS "reprappro, dc42, chrishamm, t3p3, dnewman"