From f77464412910676c0b74eec8b92d6ba3b3ebcd83 Mon Sep 17 00:00:00 2001 From: Eugene Kabanov Date: Sat, 6 Mar 2021 00:22:32 +0200 Subject: [PATCH] Fix integer decoding overflow issue. (#163) Switch to stew.base10 procedures. Adjust tests to follow new behavior. Bump version to 2.6.1. --- chronos.nimble | 2 +- chronos/apps/http/httptable.nim | 61 ++++++++------------------------- tests/testhttpserver.nim | 23 ++++++++----- 3 files changed, 31 insertions(+), 55 deletions(-) diff --git a/chronos.nimble b/chronos.nimble index 3bd6b38..89e93af 100644 --- a/chronos.nimble +++ b/chronos.nimble @@ -1,5 +1,5 @@ packageName = "chronos" -version = "2.6.0" +version = "2.6.1" author = "Status Research & Development GmbH" description = "Chronos" license = "Apache License 2.0 or MIT" diff --git a/chronos/apps/http/httptable.nim b/chronos/apps/http/httptable.nim index 59f54a0..496e467 100644 --- a/chronos/apps/http/httptable.nim +++ b/chronos/apps/http/httptable.nim @@ -8,6 +8,7 @@ # Apache License, version 2.0, (LICENSE-APACHEv2) # MIT license (LICENSE-MIT) import std/[tables, strutils] +import stew/base10 {.push raises: [Defect].} @@ -19,40 +20,6 @@ type HttpTables* = HttpTable | HttpTableRef -proc `-`(x: uint32): uint32 {.inline.} = - (0xFFFF_FFFF'u32 - x) + 1'u32 - -proc LT(x, y: uint32): uint32 {.inline.} = - let z = x - y - (z xor ((y xor x) and (y xor z))) shr 31 - -proc decValue(c: byte): int = - # Procedure returns values [0..9] for character [`0`..`9`] and -1 for all - # other characters. - let x = uint32(c) - 0x30'u32 - let r = ((x + 1'u32) and -LT(x, 10)) - int(r) - 1 - -proc bytesToDec*[T: byte|char](src: openarray[T]): uint64 = - var v = 0'u64 - for i in 0 ..< len(src): - let d = - when T is byte: - decValue(src[i]) - else: - decValue(byte(src[i])) - if d < 0: - # non-decimal character encountered - return v - else: - let nv = ((v shl 3) + (v shl 1)) + uint64(d) - if nv < v: - # overflow happened - return 0xFFFF_FFFF_FFFF_FFFF'u64 - else: - v = nv - v - proc add*(ht: var HttpTables, key: string, value: string) = ## Add string ``value`` to header with key ``key``. var default: seq[string] @@ -101,17 +68,15 @@ proc getInt*(ht: HttpTables, key: string): uint64 = ## Integers are parsed in safe way, there no exceptions or errors will be ## raised. ## - ## If a non-decimal character is encountered during the parsing of the string - ## the current accumulated value will be returned. So if string starts with - ## non-decimal character, procedure will always return `0` (for example "-1" - ## will be decoded as `0`). But if non-decimal character will be encountered - ## later, only decimal part will be decoded, like `1234_5678` will be decoded - ## as `1234`. - ## Also, if in the parsing process result exceeds `uint64` maximum allowed - ## value, then `0xFFFF_FFFF_FFFF_FFFF'u64` will be returned (for example - ## `18446744073709551616` will be decoded as `18446744073709551615` because it - ## overflows uint64 maximum value of `18446744073709551615`). - bytesToDec(ht.getString(key)) + ## Procedure returns `0` value in next cases: + ## 1. The value is empty. + ## 2. Non-decimal character encountered during the parsing of the value. + ## 3. Result exceeds `uint64` maximum allowed value. + let res = Base10.decode(uint64, ht.getString(key)) + if res.isOk(): + res.get() + else: + 0'u64 proc getLastString*(ht: HttpTables, key: string): string = ## Returns "last" value of header ``key``. @@ -132,7 +97,11 @@ proc getLastInt*(ht: HttpTables, key: string): uint64 = ## encountered header will be returned. ## ## Unsigned integer will be parsed using rules of getInt() procedure. - bytesToDec(ht.getLastString()) + let res = Base10.decode(uint64, ht.getLastString(key)) + if res.isOk(): + res.get() + else: + 0'u64 proc init*(htt: typedesc[HttpTable]): HttpTable = ## Create empty HttpTable. diff --git a/tests/testhttpserver.nim b/tests/testhttpserver.nim index 4562611..a57b174 100644 --- a/tests/testhttpserver.nim +++ b/tests/testhttpserver.nim @@ -7,6 +7,7 @@ # MIT license (LICENSE-MIT) import std/[strutils, unittest, algorithm, strutils] import ../chronos, ../chronos/apps +import stew/base10 # To create self-signed certificate and key you can use openssl # openssl req -new -x509 -sha256 -newkey rsa:2048 -nodes \ @@ -431,7 +432,7 @@ suite "HTTP server testing suite": let message = "POST / HTTP/1.0\r\n" & "Content-Type: application/x-www-form-urlencoded\r\n" & - "Content-Length: 20" & + "Content-Length: 20\r\n" & "Cookie: 2\r\n\r\n" & "a=a&b=b&c=c&d=%D0%9F" let data = await httpClient(address, message) @@ -811,19 +812,25 @@ suite "HTTP server testing suite": ("", 0'u64), ("0", 0'u64), ("-0", 0'u64), ("0-", 0'u64), ("01", 1'u64), ("001", 1'u64), ("0000000000001", 1'u64), ("18446744073709551615", 0xFFFF_FFFF_FFFF_FFFF'u64), - ("18446744073709551616", 0xFFFF_FFFF_FFFF_FFFF'u64), - ("99999999999999999999", 0xFFFF_FFFF_FFFF_FFFF'u64), - ("999999999999999999999999999999999999", 0xFFFF_FFFF_FFFF_FFFF'u64), + ("18446744073709551616", 0'u64), + ("99999999999999999999", 0'u64), + ("999999999999999999999999999999999999", 0'u64), ("FFFFFFFFFFFFFFFF", 0'u64), - ("0123456789ABCDEF", 123456789'u64) + ("0123456789ABCDEF", 0'u64) ] for i in 0 ..< 256: + let res = Base10.decode(uint64, [char(i)]) if char(i) in {'0' .. '9'}: - check bytesToDec($char(i)) == uint64(i - ord('0')) + check: + res.isOk() + res.get() == uint64(i - ord('0')) else: - check bytesToDec($char(i)) == 0'u64 + check res.isErr() + for item in TestVectors: - check bytesToDec(item[0]) == item[1] + var ht = HttpTable.init([("test", item[0])]) + let value = ht.getInt("test") + check value == item[1] test "HttpTable behavior test": var table1 = HttpTable.init()