Re: [ovs-dev] [PATCH] ofp-parse: Fix bug that doesn't stop at null byte

2018-12-03 Thread Ben Pfaff
On Tue, Nov 27, 2018 at 04:10:10PM -0800, Yifeng Sun wrote:
> At the end of string s when s[n] == '\0', strchr(delimiters, '\0')
> returns a non-null value. As a result, this function reads beyond
> the valid length of s and returns an erroneous length. This patch
> fixes it.
> 
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11473
> Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11505
> Signed-off-by: Yifeng Sun 
> ---
>  lib/ofp-parse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index a8b5a877c59e..fd008dd80e7d 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -259,7 +259,7 @@ parse_value(const char *s, const char *delimiters)
>   *
>   * strchr(s, '\0') returns s+strlen(s), so this test handles the null
>   * terminator at the end of 's'.  */
> -while (!strchr(delimiters, s[n])) {
> +while (s[n] != '\0' && !strchr(delimiters, s[n])) {

Thanks for the fix, but it doesn't make sense to me.  As you said in the
commit message, and as it says in the comment just above the line that
the patch modifies, strchr(delimiters, '\0') returns nonnull.  When it
returns nonnull, the loop condition is false and the body of the loop
doesn't execute.  I guess I must be missing something.  What am I
missing?

Thanks,

Ben.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH] ofp-parse: Fix bug that doesn't stop at null byte

2018-11-27 Thread Yifeng Sun
At the end of string s when s[n] == '\0', strchr(delimiters, '\0')
returns a non-null value. As a result, this function reads beyond
the valid length of s and returns an erroneous length. This patch
fixes it.

Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11473
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=11505
Signed-off-by: Yifeng Sun 
---
 lib/ofp-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index a8b5a877c59e..fd008dd80e7d 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -259,7 +259,7 @@ parse_value(const char *s, const char *delimiters)
  *
  * strchr(s, '\0') returns s+strlen(s), so this test handles the null
  * terminator at the end of 's'.  */
-while (!strchr(delimiters, s[n])) {
+while (s[n] != '\0' && !strchr(delimiters, s[n])) {
 if (s[n] == '(') {
 int level = 0;
 do {
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev