You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2018/07/30 16:34:55 UTC
[1/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Validate compound type lengths more carefully against incoming
buffer size
Repository: qpid-proton
Updated Branches:
refs/heads/master fdb3789b9 -> d722f7df5
PROTON-1903: Bug found by OSS Fuzz project
- Validate compound type lengths more carefully against incoming buffer size
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8303
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/f48f4c5f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/f48f4c5f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/f48f4c5f
Branch: refs/heads/master
Commit: f48f4c5f6a69f098820a3972377277dabf1df24f
Parents: fdb3789
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:04:44 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:33:44 2018 -0400
----------------------------------------------------------------------
c/src/core/decoder.c | 17 +++++++++++++----
.../fuzz-message-decode/crash/5311329584807936 | Bin 0 -> 3 bytes
2 files changed, 13 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f48f4c5f/c/src/core/decoder.c
----------------------------------------------------------------------
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index f56b275..a88e10a 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -360,22 +360,31 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
case PNE_LIST8:
case PNE_LIST32:
case PNE_MAP8:
- case PNE_MAP32:
+ case PNE_MAP32: {
+ size_t min_expected_size = 0;
switch (code)
{
case PNE_ARRAY8:
+ min_expected_size += 1; // Array has a constructor of at least 1 byte
case PNE_LIST8:
case PNE_MAP8:
- if (pn_decoder_remaining(decoder) < 2) return PN_UNDERFLOW;
+ min_expected_size += 1; // All these types have a count
+ if (pn_decoder_remaining(decoder) < min_expected_size+1) return PN_UNDERFLOW;
size = pn_decoder_readf8(decoder);
+ // size must be at least big enough for count or count+constructor
+ if (size < min_expected_size) return PN_ARG_ERR;
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
count = pn_decoder_readf8(decoder);
break;
case PNE_ARRAY32:
+ min_expected_size += 1; // Array has a constructor of at least 1 byte
case PNE_LIST32:
case PNE_MAP32:
- if (pn_decoder_remaining(decoder) < 8) return PN_UNDERFLOW;
+ min_expected_size += 4; // All these types have a count
+ if (pn_decoder_remaining(decoder) < min_expected_size+4) return PN_UNDERFLOW;
size = pn_decoder_readf32(decoder);
+ // size must be at least big enough for count or count+constructor
+ if (size < min_expected_size) return PN_ARG_ERR;
if (pn_decoder_remaining(decoder) < size) return PN_UNDERFLOW;
count = pn_decoder_readf32(decoder);
break;
@@ -422,7 +431,6 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
default:
return PN_ARG_ERR;
}
-
pn_data_enter(data);
for (size_t i = 0; i < count; i++)
{
@@ -432,6 +440,7 @@ static int pni_decoder_decode_value(pn_decoder_t *decoder, pn_data_t *data, uint
pn_data_exit(data);
return 0;
+ }
default:
return pn_error_format(decoder->error, PN_ARG_ERR, "unrecognized typecode: %u", code);
}
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/f48f4c5f/c/tests/fuzz/fuzz-message-decode/crash/5311329584807936
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-message-decode/crash/5311329584807936 b/c/tests/fuzz/fuzz-message-decode/crash/5311329584807936
new file mode 100644
index 0000000..d2ab94f
Binary files /dev/null and b/c/tests/fuzz/fuzz-message-decode/crash/5311329584807936 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[7/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Fix sasl code to disallow some illegal frame sequences
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Fix sasl code to disallow some illegal frame sequences
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8304
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/9fbd8abe
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/9fbd8abe
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/9fbd8abe
Branch: refs/heads/master
Commit: 9fbd8abe863180b0a071e24a95e443761c0df343
Parents: 1cce7ca
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:02:56 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:41:59 2018 -0400
----------------------------------------------------------------------
c/src/sasl/sasl.c | 43 ++++++++++++++++++-
.../crash/6028258679193600 | Bin 0 -> 28 bytes
2 files changed, 42 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9fbd8abe/c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 8366116..3b257ab 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -847,6 +847,14 @@ pn_sasl_outcome_t pn_sasl_outcome(pn_sasl_t *sasl0)
int pn_do_init(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
pni_sasl_t *sasl = transport->sasl;
+
+ // If we haven't got an sasl struct yet we've in an error state
+ // This can only happen if our peer sent SASL frames even though he didn't send the SASL header
+ if (!sasl) return PN_ERR;
+
+ // We should only receive this if we are a sasl server
+ if (sasl->client) return PN_ERR;
+
pn_bytes_t mech;
pn_bytes_t recv;
int err = pn_data_scan(args, "D.[sz]", &mech, &recv);
@@ -872,6 +880,13 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
{
pni_sasl_t *sasl = transport->sasl;
+ // If we haven't got an sasl struct yet we've in an error state
+ // This can only happen if our peer sent SASL frames even though we didn't send the SASL header
+ if (!sasl) return PN_ERR;
+
+ // We should only receive this if we are a sasl client
+ if (!sasl->client) return PN_ERR;
+
// This scanning relies on pn_data_scan leaving the pn_data_t cursors
// where they are after finishing the scan
pn_string_t *mechs = pn_string("");
@@ -919,6 +934,15 @@ int pn_do_mechanisms(pn_transport_t *transport, uint8_t frame_type, uint16_t cha
// Received client side
int pn_do_challenge(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
+ pni_sasl_t *sasl = transport->sasl;
+
+ // If we haven't got an sasl struct yet we've in an error state
+ // This can only happen if our peer sent SASL frames even though we didn't send the SASL header
+ if (!sasl) return PN_ERR;
+
+ // We should only receive this if we are a sasl client
+ if (!sasl->client) return PN_ERR;
+
pn_bytes_t recv;
int err = pn_data_scan(args, "D.[z]", &recv);
if (err) return err;
@@ -931,6 +955,15 @@ int pn_do_challenge(pn_transport_t *transport, uint8_t frame_type, uint16_t chan
// Received server side
int pn_do_response(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
+ pni_sasl_t *sasl = transport->sasl;
+
+ // If we haven't got an sasl struct yet we've in an error state
+ // This can only happen if our peer sent SASL frames even though he didn't send the SASL header
+ if (!sasl) return PN_ERR;
+
+ // We should only receive this if we are a sasl server
+ if (sasl->client) return PN_ERR;
+
pn_bytes_t recv;
int err = pn_data_scan(args, "D.[z]", &recv);
if (err) return err;
@@ -943,11 +976,19 @@ int pn_do_response(pn_transport_t *transport, uint8_t frame_type, uint16_t chann
// Received client side
int pn_do_outcome(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
{
+ pni_sasl_t *sasl = transport->sasl;
+
+ // If we haven't got an sasl struct yet we've in an error state
+ // This can only happen if our peer sent SASL frames even though we didn't send the SASL header
+ if (!sasl) return PN_ERR;
+
+ // We should only receive this if we are a sasl client
+ if (!sasl->client) return PN_ERR;
+
uint8_t outcome;
int err = pn_data_scan(args, "D.[B]", &outcome);
if (err) return err;
- pni_sasl_t *sasl = transport->sasl;
sasl->outcome = (pn_sasl_outcome_t) outcome;
bool authenticated = sasl->outcome==PN_SASL_OK;
transport->authenticated = authenticated;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9fbd8abe/c/tests/fuzz/fuzz-connection-driver/crash/6028258679193600
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/6028258679193600 b/c/tests/fuzz/fuzz-connection-driver/crash/6028258679193600
new file mode 100644
index 0000000..1d58614
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/6028258679193600 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[3/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Fix error codes returned if there is an error in channel processing
when processing the bein performative. -- The previous returns weren't error
codes but event codes and they w
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Fix error codes returned if there is an error in channel processing
when processing the bein performative.
-- The previous returns weren't error codes but event codes and they weren't
negative so didn't register as an error.
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8306
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8319
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/6dfcf326
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/6dfcf326
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/6dfcf326
Branch: refs/heads/master
Commit: 6dfcf32691b5da1468ac2b252bda34cd8a9d1dc7
Parents: 88fed8a
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:04:03 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:36:23 2018 -0400
----------------------------------------------------------------------
c/src/core/transport.c | 4 ++--
.../fuzz-connection-driver/crash/6266408911503360 | Bin 0 -> 31 bytes
.../fuzz-connection-driver/crash/6301141305393152 | Bin 0 -> 38 bytes
3 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6dfcf326/c/src/core/transport.c
----------------------------------------------------------------------
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index ae00773..9c448d0 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -1229,7 +1229,7 @@ int pn_do_begin(pn_transport_t *transport, uint8_t frame_type, uint16_t channel,
channel,
transport->channel_max
);
- return PN_TRANSPORT_ERROR;
+ return PN_ARG_ERR;
}
pn_session_t *ssn;
@@ -1241,7 +1241,7 @@ int pn_do_begin(pn_transport_t *transport, uint8_t frame_type, uint16_t channel,
"begin reply to unknown channel %d.",
remote_channel
);
- return PN_TRANSPORT_ERROR;
+ return PN_ARG_ERR;
}
} else {
ssn = pn_session(transport->connection);
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6dfcf326/c/tests/fuzz/fuzz-connection-driver/crash/6266408911503360
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/6266408911503360 b/c/tests/fuzz/fuzz-connection-driver/crash/6266408911503360
new file mode 100644
index 0000000..e1c522e
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/6266408911503360 differ
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/6dfcf326/c/tests/fuzz/fuzz-connection-driver/crash/6301141305393152
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/6301141305393152 b/c/tests/fuzz/fuzz-connection-driver/crash/6301141305393152
new file mode 100644
index 0000000..1d4a585
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/6301141305393152 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[5/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Tell the compiler better about the string tables to avoid asam
bounds error
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Tell the compiler better about the string tables to avoid asam bounds error
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8309
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/62d37225
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/62d37225
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/62d37225
Branch: refs/heads/master
Commit: 62d372256ba539c503063c50f164d2506cdeb80a
Parents: 2f43680
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:03:41 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:40:25 2018 -0400
----------------------------------------------------------------------
c/src/core/codec.c | 4 ++--
.../fuzz-connection-driver/crash/6237435934539776 | Bin 0 -> 291 bytes
2 files changed, 2 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/62d37225/c/src/core/codec.c
----------------------------------------------------------------------
diff --git a/c/src/core/codec.c b/c/src/core/codec.c
index b4322d7..595a4e6 100644
--- a/c/src/core/codec.c
+++ b/c/src/core/codec.c
@@ -265,7 +265,7 @@ int pni_inspect_enter(void *ctx, pn_data_t *data, pni_node_t *node)
return 0;
}
const char *name = (index < grandfields->field_count)
- ? FIELD_STRINGPOOL.STRING0+FIELD_FIELDS[grandfields->first_field_index+index]
+ ? (const char*)FIELD_STRINGPOOL.STRING0+FIELD_FIELDS[grandfields->first_field_index+index]
: NULL;
if (name) {
err = pn_string_addf(str, "%s=", name);
@@ -285,7 +285,7 @@ int pni_inspect_enter(void *ctx, pn_data_t *data, pni_node_t *node)
return pn_string_addf(str, "{");
default:
if (fields && index == 0) {
- err = pn_string_addf(str, "%s", FIELD_STRINGPOOL.STRING0+FIELD_NAME[fields->name_index]);
+ err = pn_string_addf(str, "%s", (const char *)FIELD_STRINGPOOL.STRING0+FIELD_NAME[fields->name_index]);
if (err) return err;
err = pn_string_addf(str, "(");
if (err) return err;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/62d37225/c/tests/fuzz/fuzz-connection-driver/crash/6237435934539776
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/6237435934539776 b/c/tests/fuzz/fuzz-connection-driver/crash/6237435934539776
new file mode 100644
index 0000000..0a6e625
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/6237435934539776 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[6/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Forbid described types with a described type descriptor -- This
avoids a potential stack overflow with the current type decoder
implementation caused by recursion. -- Although
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Forbid described types with a described type descriptor
-- This avoids a potential stack overflow with the current type
decoder implementation caused by recursion.
-- Although the type system formally allows them they are reserved
and have no current use.
- Removed the nested descriptor type test from go as this is now forbidden
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8310
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/1cce7cab
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/1cce7cab
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/1cce7cab
Branch: refs/heads/master
Commit: 1cce7cab37513e51468be7b343e3f0942e9861d9
Parents: 62d3722
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:05:11 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:41:09 2018 -0400
----------------------------------------------------------------------
c/src/core/decoder.c | 50 ++++++++++++++-----
.../fuzz-message-decode/crash/6289534089166848 | Bin 0 -> 37748 bytes
go/src/qpid.apache.org/amqp/types_test.go | 17 -------
3 files changed, 38 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1cce7cab/c/src/core/decoder.c
----------------------------------------------------------------------
diff --git a/c/src/core/decoder.c b/c/src/core/decoder.c
index a88e10a..fd7b69a 100644
--- a/c/src/core/decoder.c
+++ b/c/src/core/decoder.c
@@ -190,6 +190,7 @@ static inline pn_type_t pn_code2type(uint8_t code)
}
static int pni_decoder_decode_type(pn_decoder_t *decoder, pn_data_t *data, uint8_t *code);
+static int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data);
static int pni_decoder_single(pn_decoder_t *decoder, pn_data_t *data);
void pni_data_set_array_type(pn_data_t *data, pn_type_t type);
@@ -460,26 +461,51 @@ static int pni_decoder_decode_type(pn_decoder_t *decoder, pn_data_t *data, uint8
uint8_t next = *decoder->position++;
- if (next == PNE_DESCRIPTOR) {
- if (pni_data_parent_type(data) != PN_ARRAY) {
- err = pn_data_put_described(data);
- if (err) return err;
- // pni_decoder_single has the corresponding exit
- pn_data_enter(data);
- }
- err = pni_decoder_single(decoder, data);
- if (err) return err;
- err = pni_decoder_decode_type(decoder, data, code);
- if (err) return err;
- } else {
+ if (next != PNE_DESCRIPTOR) {
*code = next;
+ return 0;
}
+ if (pni_data_parent_type(data) != PN_ARRAY) {
+ err = pn_data_put_described(data);
+ if (err) return err;
+
+ // pni_decoder_single has the corresponding exit
+ pn_data_enter(data);
+ }
+
+ err = pni_decoder_single_described(decoder, data);
+ if (err) return err;
+
+ err = pni_decoder_decode_type(decoder, data, code);
+ if (err) return err;
+
return 0;
}
size_t pn_data_siblings(pn_data_t *data);
+int pni_decoder_single_described(pn_decoder_t *decoder, pn_data_t *data)
+{
+ if (!pn_decoder_remaining(decoder)) {
+ return PN_UNDERFLOW;
+ }
+
+ uint8_t code = *decoder->position++;;
+
+ if (code == PNE_DESCRIPTOR) {
+ return PN_ARG_ERR;
+ }
+
+ int err = pni_decoder_decode_value(decoder, data, code);
+ if (err) return err;
+
+ if (pni_data_parent_type(data) == PN_DESCRIBED && pn_data_siblings(data) > 1) {
+ pn_data_exit(data);
+ }
+ return 0;
+}
+
int pni_decoder_single(pn_decoder_t *decoder, pn_data_t *data)
{
uint8_t code;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1cce7cab/c/tests/fuzz/fuzz-message-decode/crash/6289534089166848
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-message-decode/crash/6289534089166848 b/c/tests/fuzz/fuzz-message-decode/crash/6289534089166848
new file mode 100644
index 0000000..0fb4adc
Binary files /dev/null and b/c/tests/fuzz/fuzz-message-decode/crash/6289534089166848 differ
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1cce7cab/go/src/qpid.apache.org/amqp/types_test.go
----------------------------------------------------------------------
diff --git a/go/src/qpid.apache.org/amqp/types_test.go b/go/src/qpid.apache.org/amqp/types_test.go
index efa6e59..8f7cd6a 100644
--- a/go/src/qpid.apache.org/amqp/types_test.go
+++ b/go/src/qpid.apache.org/amqp/types_test.go
@@ -199,21 +199,4 @@ func TestDescribed(t *testing.T) {
if err := checkEqual(want.Value, s); err != nil {
t.Error(err)
}
-
- // Nested described types
- want = Described{Described{int64(123), true}, "foo"}
- marshaled, _ = Marshal(want, nil)
- if err := checkUnmarshal(marshaled, &d); err != nil {
- t.Error(err)
- }
- if err := checkEqual(want, d); err != nil {
- t.Error(err)
- }
- // Nested to interface
- if err := checkUnmarshal(marshaled, &i); err != nil {
- t.Error(err)
- }
- if err := checkEqual(want, i); err != nil {
- t.Error(err)
- }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[4/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Had to fix pn_sequnce_t to be unsigned to allow it to wrap on
increment as only unsigned int has defined behaviour on overflow. signed DOES
NOT. - Python and Ruby tests fixed to
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Had to fix pn_sequnce_t to be unsigned to allow it to wrap on increment
as only unsigned int has defined behaviour on overflow. signed DOES NOT.
- Python and Ruby tests fixed to not try negative sequence-no values as the
AMQP spec explicitly defines this to be unsigned.
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8308
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/2f436803
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/2f436803
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/2f436803
Branch: refs/heads/master
Commit: 2f43680313c761d525354bb64fa4e5be79a51abb
Parents: 6dfcf32
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:01:28 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:39:15 2018 -0400
----------------------------------------------------------------------
c/include/proton/types.h | 2 +-
c/src/core/transport.c | 15 ++++++---------
c/src/messenger/store.c | 6 +++---
.../fuzz-connection-driver/crash/5938290925502464 | Bin 0 -> 147 bytes
python/tests/proton_tests/message.py | 2 +-
ruby/spec/message_spec.rb | 8 +-------
6 files changed, 12 insertions(+), 21 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/c/include/proton/types.h
----------------------------------------------------------------------
diff --git a/c/include/proton/types.h b/c/include/proton/types.h
index 9df0cc6..2619b5c 100644
--- a/c/include/proton/types.h
+++ b/c/include/proton/types.h
@@ -135,7 +135,7 @@ extern "C" {
*
* @ingroup api_types
*/
-typedef int32_t pn_sequence_t;
+typedef uint32_t pn_sequence_t;
/**
* A span of time in milliseconds.
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/c/src/core/transport.c
----------------------------------------------------------------------
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index 9c448d0..b47f680 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -975,7 +975,7 @@ static int pni_post_amqp_transfer_frame(pn_transport_t *transport, uint16_t ch,
bool batchable)
{
bool more_flag = more;
- int framecount = 0;
+ unsigned framecount = 0;
pn_buffer_t *frame = transport->frame;
// create preformatives, assuming 'more' flag need not change
@@ -1626,13 +1626,8 @@ static int pn_scan_error(pn_data_t *data, pn_condition_t *condition, const char
return 0;
}
-/*
- This operator, inspired by code for the qpid cpp broker, gives the correct
- result when comparing sequence numbers implemented in a signed integer type.
-*/
-
-static inline int sequence_cmp(pn_sequence_t a, pn_sequence_t b) {
- return a-b;
+static inline bool sequence_lte(pn_sequence_t a, pn_sequence_t b) {
+ return b-a <= INT32_MAX;
}
int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t channel, pn_data_t *args, const pn_bytes_t *payload)
@@ -1664,7 +1659,9 @@ int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t ch
bool remote_data = (pn_data_next(transport->disp_data) &&
pn_data_get_list(transport->disp_data) > 0);
- for (pn_sequence_t id = first; sequence_cmp(id, last) <= 0; ++id) {
+ // TODO: We need to clamp the first & last values here to the actual first and last unsettled
+ // Otherwise we could just be told to process any old sequence.
+ for (pn_sequence_t id = first; sequence_lte(id, last); ++id) {
pn_delivery_t *delivery = pni_delivery_map_get(deliveries, id);
pn_disposition_t *remote = &delivery->remote;
if (delivery) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/c/src/messenger/store.c
----------------------------------------------------------------------
diff --git a/c/src/messenger/store.c b/c/src/messenger/store.c
index 44f24f1..6f6e772 100644
--- a/c/src/messenger/store.c
+++ b/c/src/messenger/store.c
@@ -39,7 +39,7 @@ struct pni_store_t {
pni_entry_t *store_tail;
pn_hash_t *tracked;
size_t size;
- int window;
+ unsigned window;
pn_sequence_t lwm;
pn_sequence_t hwm;
};
@@ -348,7 +348,7 @@ pni_entry_t *pni_store_entry(pni_store_t *store, pn_sequence_t id)
bool pni_store_tracking(pni_store_t *store, pn_sequence_t id)
{
- return (id - store->lwm >= 0) && (store->hwm - id > 0);
+ return (id - store->lwm < INT32_MAX) && (store->hwm - id < INT32_MAX + 1u);
}
pn_sequence_t pni_entry_track(pni_entry_t *entry)
@@ -359,7 +359,7 @@ pn_sequence_t pni_entry_track(pni_entry_t *entry)
entry->id = store->hwm++;
pn_hash_put(store->tracked, entry->id, entry);
- if (store->window >= 0) {
+ if (store->window < INT32_MAX) {
while (store->hwm - store->lwm > store->window) {
pni_entry_t *e = pni_store_entry(store, store->lwm);
if (e) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/c/tests/fuzz/fuzz-connection-driver/crash/5938290925502464
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/5938290925502464 b/c/tests/fuzz/fuzz-connection-driver/crash/5938290925502464
new file mode 100644
index 0000000..411b30d
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/5938290925502464 differ
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/python/tests/proton_tests/message.py
----------------------------------------------------------------------
diff --git a/python/tests/proton_tests/message.py b/python/tests/proton_tests/message.py
index 26a3dd2..cb8c567 100644
--- a/python/tests/proton_tests/message.py
+++ b/python/tests/proton_tests/message.py
@@ -99,7 +99,7 @@ class AccessorsTest(Test):
self._test_str("group_id")
def testGroupSequence(self):
- self._test("group_sequence", 0, (0, -10, 10, 20, -20))
+ self._test("group_sequence", 0, (0, 1, 10, 20, 4294967294, 4294967295))
def testReplyToGroupId(self):
self._test_str("reply_to_group_id")
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/2f436803/ruby/spec/message_spec.rb
----------------------------------------------------------------------
diff --git a/ruby/spec/message_spec.rb b/ruby/spec/message_spec.rb
index 5383f5a..7976716 100644
--- a/ruby/spec/message_spec.rb
+++ b/ruby/spec/message_spec.rb
@@ -331,19 +331,13 @@ module Qpid
}.must_raise(TypeError)
end
- it "can have a negative group sequence" do
- seq = (0 - rand(32767))
- @message.group_sequence = seq
- @message.group_sequence.must_equal(seq)
- end
-
it "can have a zero group sequence" do
@message.group_sequence = 0
@message.group_sequence.must_equal(0)
end
it "has a group sequence" do
- id = rand(32767)
+ id = rand(4294967295)
@message.group_sequence = id
@message.group_sequence.must_equal(id)
end
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[8/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Don't go off into the weeds if our peer tries to set the
disposition of arbirtrary sequence nos.
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Don't go off into the weeds if our peer tries to set the disposition of arbirtrary
sequence nos.
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8307
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/d722f7df
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/d722f7df
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/d722f7df
Branch: refs/heads/master
Commit: d722f7df5c714074b07cd803cbd6406edca4b40c
Parents: 9fbd8ab
Author: Andrew Stitcher <as...@apache.org>
Authored: Mon Jul 30 12:20:57 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 12:20:57 2018 -0400
----------------------------------------------------------------------
c/src/core/transport.c | 9 ++++++---
.../fuzz-connection-driver/crash/5092805675319296 | Bin 0 -> 186 bytes
2 files changed, 6 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/d722f7df/c/src/core/transport.c
----------------------------------------------------------------------
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index b47f680..106a2c8 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -1659,12 +1659,15 @@ int pn_do_disposition(pn_transport_t *transport, uint8_t frame_type, uint16_t ch
bool remote_data = (pn_data_next(transport->disp_data) &&
pn_data_get_list(transport->disp_data) > 0);
- // TODO: We need to clamp the first & last values here to the actual first and last unsettled
- // Otherwise we could just be told to process any old sequence.
+ // Do some validation of received first and last values
+ // TODO: We should really also clamp the first value here, but we're not keeping track of the earliest
+ // unsettled delivery sequence no
+ last = sequence_lte(last, deliveries->next) ? last : deliveries->next;
+ first = sequence_lte(first, last) ? first : last;
for (pn_sequence_t id = first; sequence_lte(id, last); ++id) {
pn_delivery_t *delivery = pni_delivery_map_get(deliveries, id);
- pn_disposition_t *remote = &delivery->remote;
if (delivery) {
+ pn_disposition_t *remote = &delivery->remote;
if (type_init) remote->type = type;
if (remote_data) {
switch (type) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/d722f7df/c/tests/fuzz/fuzz-connection-driver/crash/5092805675319296
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/5092805675319296 b/c/tests/fuzz/fuzz-connection-driver/crash/5092805675319296
new file mode 100644
index 0000000..f460869
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/5092805675319296 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org
[2/8] qpid-proton git commit: PROTON-1903: Bug found by OSS Fuzz
project - Avoid calling pn_transport_capacity() during protocol processing,
as it can reallocate the data buffer thus invalidating the data pointer
Posted by as...@apache.org.
PROTON-1903: Bug found by OSS Fuzz project
- Avoid calling pn_transport_capacity() during protocol processing, as it can
reallocate the data buffer thus invalidating the data pointer
OSS-Fuzz bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8305
Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/88fed8a9
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/88fed8a9
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/88fed8a9
Branch: refs/heads/master
Commit: 88fed8a9abe341ccc3cfaa3e7b871dc0489df3bc
Parents: f48f4c5
Author: Andrew Stitcher <as...@apache.org>
Authored: Sun Jul 29 00:02:20 2018 -0400
Committer: Andrew Stitcher <as...@apache.org>
Committed: Mon Jul 30 11:35:09 2018 -0400
----------------------------------------------------------------------
c/src/core/transport.c | 9 +++------
c/src/sasl/sasl.c | 11 ++++++++---
.../fuzz-connection-driver/crash/5972719047802880 | Bin 0 -> 16384 bytes
3 files changed, 11 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/88fed8a9/c/src/core/transport.c
----------------------------------------------------------------------
diff --git a/c/src/core/transport.c b/c/src/core/transport.c
index dde4b37..ae00773 100644
--- a/c/src/core/transport.c
+++ b/c/src/core/transport.c
@@ -271,7 +271,7 @@ void pn_set_error_layer(pn_transport_t *transport)
ssize_t pn_io_layer_input_autodetect(pn_transport_t *transport, unsigned int layer, const char *bytes, size_t available)
{
const char* error;
- bool eos = pn_transport_capacity(transport)==PN_EOS;
+ bool eos = transport->tail_closed;
if (eos && available==0) {
pn_do_error(transport, "amqp:connection:framing-error", "No valid protocol header found");
pn_set_error_layer(transport);
@@ -2533,7 +2533,7 @@ static void pn_error_amqp(pn_transport_t* transport, unsigned int layer)
static ssize_t pn_input_read_amqp_header(pn_transport_t* transport, unsigned int layer, const char* bytes, size_t available)
{
- bool eos = pn_transport_capacity(transport)==PN_EOS;
+ bool eos = transport->tail_closed;
pni_protocol_type_t protocol = pni_sniff_header(bytes, available);
switch (protocol) {
case PNI_PROTOCOL_AMQP1:
@@ -2575,10 +2575,7 @@ static ssize_t pn_input_read_amqp(pn_transport_t* transport, unsigned int layer,
ssize_t n = pn_dispatcher_input(transport, bytes, available, true, &transport->halt);
- if (n < 0) {
- //return pn_error_set(transport->error, n, "dispatch error");
- return PN_EOS;
- } else if (transport->close_rcvd) {
+ if (n < 0 || transport->close_rcvd) {
return PN_EOS;
} else {
return n;
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/88fed8a9/c/src/sasl/sasl.c
----------------------------------------------------------------------
diff --git a/c/src/sasl/sasl.c b/c/src/sasl/sasl.c
index 125c06d..8366116 100644
--- a/c/src/sasl/sasl.c
+++ b/c/src/sasl/sasl.c
@@ -541,7 +541,7 @@ static void pn_error_sasl(pn_transport_t* transport, unsigned int layer)
static ssize_t pn_input_read_sasl_header(pn_transport_t* transport, unsigned int layer, const char* bytes, size_t available)
{
- bool eos = pn_transport_capacity(transport)==PN_EOS;
+ bool eos = transport->tail_closed;
pni_protocol_type_t protocol = pni_sniff_header(bytes, available);
switch (protocol) {
case PNI_PROTOCOL_AMQP_SASL:
@@ -581,7 +581,7 @@ static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer,
{
pni_sasl_t *sasl = transport->sasl;
- bool eos = pn_transport_capacity(transport)==PN_EOS;
+ bool eos = transport->tail_closed;
if (eos) {
pn_do_error(transport, "amqp:connection:framing-error", "connection aborted");
pn_set_error_layer(transport);
@@ -591,7 +591,12 @@ static ssize_t pn_input_read_sasl(pn_transport_t* transport, unsigned int layer,
pni_sasl_start_server_if_needed(transport);
if (!pni_sasl_is_final_input_state(sasl)) {
- return pn_dispatcher_input(transport, bytes, available, false, &transport->halt);
+ ssize_t n = pn_dispatcher_input(transport, bytes, available, false, &transport->halt);
+ if (n < 0 || transport->close_rcvd) {
+ return PN_EOS;
+ } else {
+ return n;
+ }
}
if (!pni_sasl_is_final_output_state(sasl)) {
http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/88fed8a9/c/tests/fuzz/fuzz-connection-driver/crash/5972719047802880
----------------------------------------------------------------------
diff --git a/c/tests/fuzz/fuzz-connection-driver/crash/5972719047802880 b/c/tests/fuzz/fuzz-connection-driver/crash/5972719047802880
new file mode 100644
index 0000000..a8d08ef
Binary files /dev/null and b/c/tests/fuzz/fuzz-connection-driver/crash/5972719047802880 differ
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org