Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix base64 decode error on ARM platforms #1853

Merged
merged 2 commits into from
Feb 3, 2020

Conversation

meyerj
Copy link
Contributor

@meyerj meyerj commented Dec 22, 2019

I had the same problem as reported in #1768 when compiling the released version 1.14.3 of ros_comm on a Raspberry Pi 4 with gcc. Also here char seems to be an unsigned type and base64 decoding was broken. I assume this is the case on all ARM platforms.

I found this patch in the upstream bugtracker at https://sourceforge.net/p/libb64/bugs/2/ created in 2012 by Jakub Wilk. The last proposed patch that I applied here was submitted by Jonathan Wakely in 2016.

Only when I wanted to open a pull request here I realized that #1769 already addressed the topic. Jonathan's patch leaves the API as-is (char) and eliminates the implicit conversions between signed and unsigned types. For example, static const char decoding[] should not have negative values on implementations where char is unsigned.

The current patch (#1769) also solves the problem, but still produces sign conversion warnings on Raspbian 10 with gcc 8.3.0 and command line, while the new patch does not:

pi@raspberrypi:/home/pi/ros_ws/src/ros_comm/xmlrpcpp/libb64/src $ lsb_release -a
No LSB modules are available.
Distributor ID:	Raspbian
Description:	Raspbian GNU/Linux 10 (buster)
Release:	10
Codename:	buster
pi@raspberrypi:/home/pi/ros_ws/src/ros_comm/xmlrpcpp/libb64/src $ gcc --version
gcc (Raspbian 8.3.0-6+rpi1) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

pi@raspberrypi:/home/pi/ros_ws/src/ros_comm/xmlrpcpp/libb64/src $ gcc -Wall -Wextra -Werror -Wsign-conversion -pedantic -I../include -c cdecode.c
cdecode.c: In function ‘base64_decode_value’:
cdecode.c:12:37: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                     ^
cdecode.c:12:40: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                        ^
cdecode.c:12:43: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                           ^
cdecode.c:12:79: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                               ^
cdecode.c:12:82: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                  ^
cdecode.c:12:85: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                     ^
cdecode.c:12:88: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-2’ to ‘254’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                        ^
cdecode.c:12:91: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                           ^
cdecode.c:12:94: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                              ^
cdecode.c:12:97: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                 ^
cdecode.c:12:168: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                        ^
cdecode.c:12:171: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                           ^
cdecode.c:12:174: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                              ^
cdecode.c:12:177: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                                 ^
cdecode.c:12:180: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                                    ^
cdecode.c:12:183: error: unsigned conversion from ‘int’ to ‘char’ changes value from ‘-1’ to ‘255’ [-Werror=sign-conversion]
  static const char decoding[] = {62,-1,-1,-1,63,52,53,54,55,56,57,58,59,60,61,-1,-1,-1,-2,-1,-1,-1,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,-1,-1,-1,-1,-1,-1,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51};
                                                                                                                                                                                       ^
cdecode.c: In function ‘base64_decode_block’:
cdecode.c:49:42: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                                          ^~~~~~~~~~~
cdecode.c:49:16: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                ^
cdecode.c:61:42: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                                          ^~~~~~~~~~~
cdecode.c:61:16: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                ^
cdecode.c:74:42: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                                          ^~~~~~~~~~~
cdecode.c:74:16: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                ^
cdecode.c:87:42: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);
                                          ^~~~~~~~~~~
cdecode.c:87:16: error: conversion to ‘signed char’ from ‘char’ may change the sign of the result [-Werror=sign-conversion]
     fragment = (char)base64_decode_value(*codechar++);

I leave it to the maintainers to decide whether to keep the current patch by @randoms or apply this patch provided by Jakub Wilk and Jonathan Wakely instead.

Johannes Meyer and others added 2 commits December 22, 2019 01:08
Copied from https://sourceforge.net/p/libb64/bugs/2/:
> The attached patch fixes integers overflows in the decoder.
> The first hunk is needed for systems with signed chars (e.g. i386).
> The other hunks fix the decoder on unsigned-char systems (on which it's currently completely broken).
@dirk-thomas
Copy link
Member

Thanks for the patch.

@dirk-thomas dirk-thomas merged commit 61f0b76 into ros:melodic-devel Feb 3, 2020
@meyerj meyerj deleted the fix-libb64-on-arm branch February 3, 2020 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants