Permalink
Browse files

Add some error handling to parser.

  • Loading branch information...
1 parent 8bdbb70 commit e8f205e9dd7731344d232541027c3b4827016b41 @joshthecoder committed Mar 7, 2013
Showing with 91 additions and 28 deletions.
  1. +14 −12 include/irc.h
  2. +2 −1 shmooze.gyp
  3. +21 −15 src/parser.c
  4. +4 −0 test/test-list.h
  5. +48 −0 test/test-parse-illegal.c
  6. +2 −0 test/test-parse-message.c
View
@@ -6,10 +6,6 @@
typedef struct irc_message_s irc_message_t;
typedef struct irc_parser_s irc_parser_t;
-typedef enum {
- IRC_PASSWORD
-} irc_command;
-
struct irc_message_s {
const char* prefix;
const char* command;
@@ -23,22 +19,28 @@ struct irc_message_s {
*/
typedef int (*irc_message_cb)(irc_message_t* msg);
+// An illegal token was encountered while parsing message.
+#define PARSER_ILLEGAL_TOKEN 0x01
+
+// The message has exceeded the 512 byte length limit.
+#define PARSER_MESSAGE_TOO_LONG 0x02
+
struct irc_parser_s {
irc_message_cb message_cb;
- /*
- * The last error that caused the parser to stop.
- * If a non-zero value was returned from irc_message_cb
- * this variable will contain that value.
- */
+ // If an error occurred while parsing a message
+ // this will contain a non-zero error code.
+ // All errors are fatal and the connection providing
+ // the data should be terminated. The parser may only
+ // be executed again if re-initialized to clear the error.
int last_error;
- // --- Private ---
-
+ // Private.
unsigned char state;
char data[512];
- int len;
+ int data_len;
irc_message_t message;
+ int msg_len;
};
// Initialize an IRC message parser instance.
View
@@ -32,7 +32,8 @@
'test/runner.h',
'test/task.h',
'test/test-list.h',
- 'test/test-parse-message.c'
+ 'test/test-parse-message.c',
+ 'test/test-parse-illegal.c'
]
},
View
@@ -10,6 +10,9 @@ enum parser_state {
STATE_END
};
+// IRC messages cannot exceed 512 bytes.
+#define MESSAGE_MAX_LENGTH 512
+
/*
* These macros are provided for accessing the 512 byte
* buffer used by the parser for holding message parts.
@@ -26,19 +29,16 @@ void irc_parser_init(irc_parser_t* parser) {
}
int irc_parser_execute(irc_parser_t* parser, const char* buffer, size_t len) {
- // Current position in the buffer being parsed.
- const char* b = buffer;
-
- // The end position of the buffer.
- const char* b_end = b + len;
+ // Counts the number of bytes parsed from the buffer.
+ size_t bytes_parsed = 0;
unsigned char state = parser->state;
irc_message_t* message = &parser->message;
- while (b != b_end) {
+ while (bytes_parsed < len) {
// The main parser loop which runs until buffer end is reached.
// Get message character to parse and advance to next byte.
- char ch = *(b++);
+ char ch = *(buffer + bytes_parsed++);
// Process the current message byte based on current state.
switch (state) {
@@ -48,15 +48,21 @@ int irc_parser_execute(irc_parser_t* parser, const char* buffer, size_t len) {
message->parameter_count = 0;
parser->len = 0;
- if (ch == ':') {
- // Message has a prefix we need to scan.
- state = STATE_PREFIX;
- message->prefix = DATA_END(parser);
- continue;
+ switch (ch) {
+ case ':':
+ // Message has a prefix we need to scan.
+ state = STATE_PREFIX;
+ message->prefix = DATA_END(parser);
+ continue;
+
+ case ' ':
+ // Illegal to have first byte be a whitespace token.
+ parser->last_error = PARSER_ILLEGAL_TOKEN;
+ return bytes_parsed;
}
- message->command = DATA_END(parser);
- // If message has no prefix, skip to scanning the command name.
+ // If the message has no prefix, skip to parsing the command.
+ message->command = DATA_END(parser);
state = STATE_COMMAND;
break;
@@ -142,6 +148,6 @@ int irc_parser_execute(irc_parser_t* parser, const char* buffer, size_t len) {
parser->state = state;
- return len;
+ return bytes_parsed;
}
View
@@ -4,10 +4,14 @@ TEST_DECLARE (parse_message_middle)
TEST_DECLARE (parse_message_trailing)
TEST_DECLARE (parse_message_mixed)
+TEST_DECLARE (parse_illegal_first_byte_whitespace)
+
TASK_LIST_START
TEST_ENTRY (parse_message_command)
TEST_ENTRY (parse_message_prefix)
TEST_ENTRY (parse_message_middle)
TEST_ENTRY (parse_message_trailing)
TEST_ENTRY (parse_message_mixed)
+
+ TEST_ENTRY (parse_illegal_first_byte_whitespace)
TASK_LIST_END
View
@@ -0,0 +1,48 @@
+// A set of tests to verify the parser can handle illegal
+// messages by aborting and reporting error codes. These tests
+// to should guard the parser from exploits such as buffer overflow
+// by feeding it maliciously formatted messages.
+
+#include "task.h"
+
+#include "irc.h"
+
+// Parse the message and returns the last error code.
+static int parse_message(const char* msg) {
+ irc_parser_t parser;
+ irc_parser_init(&parser);
+ irc_parser_execute(&parser, msg, strlen(msg));
+ return parser.last_error;
+}
+
+// Messages cannot have whitespace as the first byte.
+// This should cause an PARSER_ILLEGAL_TOKEN error code.
+TEST_IMPL(parse_illegal_first_byte_whitespace) {
+ ASSERT(parse_message("BAD\r\n") == PARSER_ILLEGAL_TOKEN);
+ return 0;
+}
+
+// Messages that exceed 512 bytes should cause the parser
+// to abort and return error code PARSER_MESSAGE_TOO_LONG.
+TEST_IMPL(parse_illegal_too_long) {
+ const char* msg = ":Bob PRIVMSG John :";
+ size_t msg_len = strlen(msg);
+ char msg_end[1024];
+ memset(&msg_end, 'a', 1024);
+
+ irc_parser_t parser;
+ irc_parser_init(&parser);
+
+ // Seed the parser with the start of a valid message.
+ ASSERT(irc_parser_execute(&parser, msg, msg_len) == msg_len);
+ ASSERT(parser.last_error == 0);
+
+ // Feed the parser enough valid data to go over the IRC message limit.
+ // It should cause the parser to stop once it reaches 512 bytes parsed.
+ ASSERT(irc_parser_execute(&parser, msg_end, 1024) == (1024 - msg_len));
+ ASSERT(parser.last_error == PARSER_MESSAGE_TOO_LONG);
+ ASSERT(parser.len == 512);
+
+ return 0;
+}
+
@@ -1,3 +1,5 @@
+// A set of tests to verify valid IRC messages are parsed correctly.
+
#include "irc.h"
#include "task.h"

0 comments on commit e8f205e

Please sign in to comment.