Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Commit

Permalink
Closes #354 - Ignores B3 headers if invalid values are provided (#355)
Browse files Browse the repository at this point in the history
* tests for malicious input for B3TextMapCodec

Signed-off-by: Tomasz Adamski <tadamski@wikia-inc.com>

* Closes #354 - Ignores B3 headers if invalid values are provided

Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
  • Loading branch information
jpkrohling authored and pavolloffay committed Feb 27, 2018
1 parent 581e20f commit cf99e75
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void inject(SpanContext spanContext, TextMap carrier) {
public SpanContext extract(TextMap carrier) {
Long traceId = null;
Long spanId = null;
long parentId = 0L; // Conventionally, parent id == 0 means the root span
Long parentId = 0L; // Conventionally, parent id == 0 means the root span
byte flags = 0;
for (Map.Entry<String, String> entry : carrier) {
if (entry.getKey().equalsIgnoreCase(SAMPLED_NAME)) {
Expand All @@ -83,7 +83,7 @@ public SpanContext extract(TextMap carrier) {
}
}

if (traceId != null && spanId != null) {
if (null != traceId && null != parentId && null != spanId) {
return new SpanContext(traceId, spanId, parentId, flags);
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ final class HexCodec {
/**
* Parses a 1 to 32 character lower-hex string with no prefix into an unsigned long, tossing any
* bits higher than 64.
*
* @return a 64 bit long, meaning that negative values are the overflow of Java's 32 bit long
*/
static long lowerHexToUnsignedLong(String lowerHex) {
static Long lowerHexToUnsignedLong(String lowerHex) {
int length = lowerHex.length();
if (length < 1 || length > 32) {
throw isntLowerHexLong(lowerHex);
return null;
}

// trim off any high bits
Expand All @@ -36,8 +38,10 @@ static long lowerHexToUnsignedLong(String lowerHex) {
/**
* Parses a 16 character lower-hex string with no prefix into an unsigned long, starting at the
* spe index.
*
* @return a 64 bit long, meaning that negative values are the overflow of Java's 32 bit long
*/
static long lowerHexToUnsignedLong(String lowerHex, int index) {
static Long lowerHexToUnsignedLong(String lowerHex, int index) {
long result = 0;
for (int endIndex = Math.min(index + 16, lowerHex.length()); index < endIndex; index++) {
char c = lowerHex.charAt(index);
Expand All @@ -47,17 +51,12 @@ static long lowerHexToUnsignedLong(String lowerHex, int index) {
} else if (c >= 'a' && c <= 'f') {
result |= c - 'a' + 10;
} else {
throw isntLowerHexLong(lowerHex);
return null;
}
}
return result;
}

static NumberFormatException isntLowerHexLong(String lowerHex) {
throw new NumberFormatException(
lowerHex + " should be a 1 to 32 character lower-hex string with no prefix");
}

/**
* Returns 16 or 32 character hex string depending on if {@code high} is zero.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright (c) 2016, Uber Technologies, Inc
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/

package com.uber.jaeger.propagation;

import static org.junit.Assert.assertNull;

import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
import com.uber.jaeger.SpanContext;
import io.opentracing.propagation.TextMap;
import java.util.Arrays;
import org.junit.Test;
import org.junit.runner.RunWith;

@RunWith(DataProviderRunner.class)
public class B3TextMapCodecResiliencyTest {

private B3TextMapCodec sut = new B3TextMapCodec();

@DataProvider
public static Object[] maliciousInputs() {
return new Object[] {
"abcbdabcbdabcbdabcbdabcbdabcbdabcbdabcbdabcbdabcbdabcbd",
"",
"ABCDEF",
};
}

@Test
@UseDataProvider("maliciousInputs")
public void shouldFallbackWhenMaliciousInput(String maliciousInput) {
String validInput = "ffffffffffffffffffffffffffffffff";
TextMap maliciousCarrier = new B3TextMapCodecTest.DelegatingTextMap();
maliciousCarrier.put(B3TextMapCodec.TRACE_ID_NAME, validInput);
maliciousCarrier.put(B3TextMapCodec.SPAN_ID_NAME, validInput);
maliciousCarrier.put(B3TextMapCodec.PARENT_SPAN_ID_NAME, validInput);

// everything is valid, except for one of the headers at a time:
for (String header : Arrays.asList(
B3TextMapCodec.TRACE_ID_NAME,
B3TextMapCodec.SPAN_ID_NAME,
B3TextMapCodec.PARENT_SPAN_ID_NAME)) {
maliciousCarrier.put(header, maliciousInput);
//when
SpanContext extract = sut.extract(maliciousCarrier);

//then
assertNull(extract);

maliciousCarrier.put(header, validInput);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import static com.uber.jaeger.propagation.B3TextMapCodec.SPAN_ID_NAME;
import static com.uber.jaeger.propagation.B3TextMapCodec.TRACE_ID_NAME;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

import com.uber.jaeger.SpanContext;
Expand Down Expand Up @@ -59,8 +61,9 @@ public void downgrades128BitTraceIdToLower64Bits() throws Exception {

SpanContext context = b3Codec.extract(textMap);

assertEquals(HexCodec.lowerHexToUnsignedLong(lower64Bits), context.getTraceId());
assertEquals(HexCodec.lowerHexToUnsignedLong(lower64Bits), context.getSpanId());
assertNotNull(HexCodec.lowerHexToUnsignedLong(lower64Bits));
assertEquals(HexCodec.lowerHexToUnsignedLong(lower64Bits).longValue(), context.getTraceId());
assertEquals(HexCodec.lowerHexToUnsignedLong(lower64Bits).longValue(), context.getSpanId());
assertEquals(0, context.getParentId());
assertEquals(SAMPLED_FLAG | DEBUG_FLAG, context.getFlags());
}
Expand Down

0 comments on commit cf99e75

Please sign in to comment.